diff --git a/.claude/hooks/decision-nudge.js b/.agents/hooks/decision-nudge.js similarity index 85% rename from .claude/hooks/decision-nudge.js rename to .agents/hooks/decision-nudge.js index 778ed5ee0..8c4bad5ea 100755 --- a/.claude/hooks/decision-nudge.js +++ b/.agents/hooks/decision-nudge.js @@ -32,10 +32,10 @@ process.stdin.on("end", () => { const nudge = { additionalContext: - "REMINDER: Run `deno fmt` and `deno lint` after TS edits. " + - "Run `cd apps/services && make fix` after Go edits. " + + "REMINDER: Run `pnpm fmt` (oxfmt), `pnpm lint` (oxlint) and `pnpm check` (tsc) after TS edits. " + + "Run `make fix` at the repository root after Go edits. " + "If this change is an architectural decision (technology choice, API design, " + - "approach preference), consider logging it in an ADR under etc/adrs/.", + "approach preference), consider logging it in an ADR under docs/adr/.", }; process.stdout.write(JSON.stringify(nudge)); diff --git a/.agents/settings.json b/.agents/settings.json new file mode 100644 index 000000000..674169146 --- /dev/null +++ b/.agents/settings.json @@ -0,0 +1,78 @@ +{ + "permissions": { + "allow": [ + "Bash(cat:*)", + "Bash(docker:*)", + "Bash(docker compose:*)", + "Bash(find:*)", + "Bash(gh:*)", + "Bash(head:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git show:*)", + "Bash(git status:*)", + "Bash(grep:*)", + "Bash(ls:*)", + "Bash(make:*)", + "Bash(node:*)", + "Bash(oxfmt:*)", + "Bash(oxlint:*)", + "Bash(pnpm bench:*)", + "Bash(pnpm check:*)", + "Bash(pnpm cli:*)", + "Bash(pnpm exec oxfmt:*)", + "Bash(pnpm exec oxlint:*)", + "Bash(pnpm exec tsc:*)", + "Bash(pnpm fmt:*)", + "Bash(pnpm lint:*)", + "Bash(pnpm run:*)", + "Bash(pnpm test:*)", + "Bash(sort:*)", + "Bash(tail:*)", + "Bash(test:*)", + "Bash(tree:*)", + "Bash(tsc:*)", + "Bash(uniq:*)", + "Bash(wc:*)", + "mcp__plugin_context7_context7__get-library-docs", + "mcp__plugin_context7_context7__query-docs", + "mcp__plugin_context7_context7__resolve-library-id", + "Skill(architecture-guidelines)", + "Skill(coding-practices)", + "Skill(design-principles)", + "Skill(enrichment)", + "Skill(eser-rules-manager)", + "Skill(frontend-practices)", + "Skill(go-practices)", + "Skill(javascript-practices)", + "Skill(release-and-ci)", + "Skill(security-practices)", + "Skill(tooling-standards)", + "Skill(workflow-practices)", + "WebFetch(domain:api.github.com)", + "WebFetch(domain:github.com)", + "WebFetch(domain:raw.githubusercontent.com)", + "WebSearch" + ], + "deny": [ + "Bash(git add:*)", + "Bash(git branch:*)", + "Bash(git checkout:*)", + "Bash(git commit:*)", + "Bash(git fetch:*)", + "Bash(git merge:*)", + "Bash(git pull:*)", + "Bash(git push:*)", + "Bash(git rebase:*)", + "Bash(git reset:*)", + "Bash(git revert:*)", + "Bash(git stash:*)", + "Bash(git tag:*)" + ] + }, + "enabledPlugins": { + "context7@claude-plugins-official": true, + "frontend-design@claude-plugins-official": true + }, + "alwaysThinkingEnabled": true +} diff --git a/.agents/skills/architecture-guidelines/SKILL.md b/.agents/skills/architecture-guidelines/SKILL.md new file mode 100644 index 000000000..fa773b30b --- /dev/null +++ b/.agents/skills/architecture-guidelines/SKILL.md @@ -0,0 +1,34 @@ +--- +name: architecture-guidelines +description: "Design conventions of this codebase for TypeScript and Go packages and services: double-layered hexagonal architecture, explicit adapter composition, runtime access as an adapter, public API and CLI surface, request metadata, testing strategy, ADRs. Use when designing a package or service, adding options or flags, planning tests or writing an ADR. Not for file naming (use tooling-standards)." +--- + +# Architecture Guidelines + +How packages and services in this codebase are structured, composed, tested and +documented as decisions. + +## Always + +- Domain logic and its port types live together in `domain/`; adapters live in + `adapters/`. No `ports/` directory. +- Callers import adapters explicitly and pass them in. No string-selected + adapters, no convenience factories, no dynamic imports inside a package. +- Library code reaches the runtime through an adapter + (`@eserstack/standards/cross-runtime`), never `process.*` or `node:fs` + directly. +- Every option has a safe default; add an option only for a named case; no + positional booleans. +- Request ids and trace state travel in the request context, never as extra + parameters. +- Unit tests with fakes for logic, integration tests for adapters; failure paths + tested; coverage never drops. +- A lasting architectural decision gets an ADR, written with the change. + +## References + +| File | Read when | +| ----------------------------------------------------------------- | --------------------------------------------------------------------------- | +| [hexagonal-architecture.md](references/hexagonal-architecture.md) | Structuring a package, adding an adapter, adding options, flags or env vars | +| [testing.md](references/testing.md) | Deciding what and how to test, choosing the runner, coverage | +| [adrs.md](references/adrs.md) | Recording an architectural decision | diff --git a/.agents/skills/architecture-guidelines/evals/triggers.json b/.agents/skills/architecture-guidelines/evals/triggers.json new file mode 100644 index 000000000..a28c83c79 --- /dev/null +++ b/.agents/skills/architecture-guidelines/evals/triggers.json @@ -0,0 +1,62 @@ +[ + { + "query": "design a new package for background jobs, where do ports and adapters go", + "should_trigger": true + }, + { + "query": "should this CLI flag be public or hidden", + "should_trigger": true + }, + { + "query": "write an ADR for switching the bundler", + "should_trigger": true + }, + { + "query": "what should we unit test vs integration test for the http package", + "should_trigger": true + }, + { + "query": "how do request ids travel between the noskills server and worker", + "should_trigger": true + }, + { + "query": "yeni bir servis tasarlıyorum, katmanları nasıl ayırayım", + "should_trigger": true + }, + { + "query": "we're adding an option to the public API of @eserstack/config, is that ok", + "should_trigger": true + }, + { + "query": "the Go tests fail with a data race", + "should_trigger": false + }, + { + "query": "use namespace import here", + "should_trigger": false + }, + { + "query": "bump all packages to the next minor", + "should_trigger": false + }, + { + "query": "write a README for laroux", + "should_trigger": false + }, + { + "query": "set the GitHub workflow timeout", + "should_trigger": false + }, + { + "query": "should this function take a pointer or a value", + "should_trigger": false + }, + { + "query": "secret key is in the logs, fix it", + "should_trigger": false + }, + { + "query": "commit and push", + "should_trigger": false + } +] diff --git a/.agents/skills/architecture-guidelines/references/adrs.md b/.agents/skills/architecture-guidelines/references/adrs.md new file mode 100644 index 000000000..0e66e102d --- /dev/null +++ b/.agents/skills/architecture-guidelines/references/adrs.md @@ -0,0 +1,28 @@ +# Architectural Decision Records + +When to write an ADR and what it contains. + +--- + +## Architectural Decision Records + +Scope: Decisions that change a package's architecture, a public contract, a +dependency with lasting cost, or a product target + +Rule: Record the decision in this codebase's ADR directory (`docs/adr/` by +default) as `NNNN-kebab-title.md`, numbered after the highest existing file. An +ADR is written before or with the change, not after. It states: + +- `## Status`: Proposed, Accepted, or Superseded by a named ADR. +- `## Context and Problem Statement`: the forces and the question being decided. +- `## Decision Drivers`: what the decision optimizes for. +- `## Considered Options`, with the trade-offs of each. +- `## Decision Outcome`: the choice and why it beat the others. +- `## Consequences`: what becomes easier and what becomes harder. +- `## Links` and related files, when there are any. + +An accepted ADR is not rewritten to match a later decision. A new ADR supersedes +it, and the old one's status names the new one. + +**Why:** the reasoning behind a structure is what a later change needs, and it +is the part the code cannot show. diff --git a/.agents/skills/architecture-guidelines/references/hexagonal-architecture.md b/.agents/skills/architecture-guidelines/references/hexagonal-architecture.md new file mode 100644 index 000000000..ab085dd36 --- /dev/null +++ b/.agents/skills/architecture-guidelines/references/hexagonal-architecture.md @@ -0,0 +1,181 @@ +# Hexagonal Architecture and Public Surface + +How packages separate domain logic from adapters, how callers compose them, and +how public APIs and request metadata are shaped. + +## Contents + +- Hexagonal Architecture (Double-Layered) +- Runtime Access Through cross-runtime +- Public API and CLI Surface +- Request-Scoped Metadata Across Boundaries + +--- + +## Hexagonal Architecture (Double-Layered) + +Scope: Packages with external dependencies (TypeScript; the Go form is in +go-practices) + +Rule: The domain layer holds business logic and its port types together; there +is no separate `ports/` directory. Adapters implement the domain types and live +in `adapters/`. The caller composes by importing the adapters it wants and +passing them in as parameters. No string-based adapter selection, no convenience +factories that pick adapters for the caller, no dynamic imports inside the +package. + +``` +package/ +├── domain/ # Business logic + port types +│ ├── mod.ts +│ ├── bundler.ts # Bundler type + BundleData +│ ├── framework-plugin.ts # FrameworkPlugin type + noopPlugin +│ └── build-cache.ts # BuildCache type + implementation +├── adapters/ +│ ├── react/ # implements FrameworkPlugin +│ ├── tailwindcss/ # implements CssPlugin +│ └── prebuilt-bundler/ # implements Bundler +└── mod.ts +``` + +Correct: + +```typescript +import * as bundler from "@scope/bundler"; +import * as react from "@scope/bundler/adapters/react"; +import * as tailwind from "@scope/bundler/adapters/tailwindcss"; +import * as prebuilt from "@scope/bundler/adapters/prebuilt-bundler"; + +await bundler.bundle(config, { + framework: react.reactPlugin, + css: tailwind.tailwindPlugin, + bundler: new prebuilt.PrebuiltBundler(bundlerConfig), +}); +``` + +Incorrect: + +```typescript +await bundle({ framework: "react", css: "tailwindcss" }); // dynamic imports inside + +const plugins = await createReactTailwindPlugins(); // hides which adapters load + +import type { Plugin } from "./ports/plugin.ts"; // ports split from the domain +``` + +A domain type ships with a noop implementation for callers that do not need the +capability: + +```typescript +// domain/framework-plugin.ts +export type FrameworkPlugin = { + name: string; + analyzeClientComponents?: (srcDir: string) => Promise; +}; + +export const noopPlugin: FrameworkPlugin = { + name: "noop", + analyzeClientComponents: () => Promise.resolve([]), +}; +``` + +**Why:** explicit imports keep the dependency graph visible and tree-shakable, +and let a caller swap one adapter without touching the others. + +--- + +## Runtime Access Through cross-runtime + +Scope: All TypeScript library code + +Rule: Filesystem, process, exec and environment access goes through a runtime +adapter, `@eserstack/standards/cross-runtime`, never Node's `process` or +`node:fs` directly. The runtime is an adapter like any other: tests pass a fake, +and code that needs an API the adapter lacks adds it there instead of calling +the runtime directly. The call-site rules are in javascript-practices: +Cross-Runtime APIs. + +Correct: + +```typescript +import { runtime } from "@eserstack/standards/cross-runtime"; + +const content = await runtime.fs.readTextFile(path); +``` + +Incorrect: + +```typescript +import * as fs from "node:fs/promises"; + +const content = await fs.readFile(path, "utf8"); // ties the library to Node +``` + +**Why:** the adapter keeps the runtime replaceable in one place, so a later +change of the runtime layer, such as collapsing the adapter into `node:*` and +Web APIs, does not touch the libraries that use it (ADR 0005). + +--- + +## Public API and CLI Surface + +Scope: Exported functions and types, CLI commands and flags, config keys, +environment variables + +Rule: + +- The common case works with no configuration. Every option has a default that + is safe and correct for production. +- A fixed policy value a caller may need to change (a timeout, a size limit, a + path, a retry count) is a parameter, flag or environment variable with that + default, not a hardcoded constant. +- Do not add options for hypothetical needs. Each new option names the case that + requires it; an edge case gets documentation, not a required setting. +- No positional boolean parameters. Use an options object or a string union. +- Defaults come from parameter defaults and noop adapters, not from convenience + factories. + +Correct: + +```typescript +export function connect(url: string, options: { timeoutMs?: number } = {}) { + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; +} + +render(tree, { mode: "stream" }); +``` + +Incorrect: + +```typescript +export function connect(url: string) { + const timeoutMs = 5000; // caller cannot change it +} + +render(tree, true, false); // what are true and false? +``` + +--- + +## Request-Scoped Metadata Across Boundaries + +Scope: HTTP and RPC servers and clients, daemons, services that call each other + +Rule: Metadata that identifies a request (trace and span ids, a request or +correlation id, the authenticated principal) travels with the request's context +(`context.Context` in Go, the request or an explicit context object in +TypeScript), not as extra function parameters. Business inputs and dependencies +never travel that way: inputs are parameters, dependencies come from the +composition root. + +- At an entry point, middleware reads incoming metadata or creates it, and + stores it in the context. +- On every outbound call, the client writes it back out, so logs and traces on + both sides join up. +- Trace state uses OpenTelemetry propagation (the `traceparent` header), not + hand-written headers. Other ids use typed accessors owned by one package. +- Logs include these ids (coding-practices: Structured Logging). + +**Why:** a request that loses its ids at one hop cannot be followed across the +system, and a dependency hidden in a context is invisible in every signature +that needs it. diff --git a/.agents/skills/architecture-guidelines/references/testing.md b/.agents/skills/architecture-guidelines/references/testing.md new file mode 100644 index 000000000..02a5497e1 --- /dev/null +++ b/.agents/skills/architecture-guidelines/references/testing.md @@ -0,0 +1,123 @@ +# Testing Strategy + +What gets tested, at which level, with which runner, and how coverage is +tracked. Language details: javascript-practices and go-practices. + +## Contents + +- What to Test +- Runner, Files and Coverage +- Table-Driven Tests + +--- + +## What to Test + +Scope: All code changes + +Rule: + +- Business logic has unit tests that use fake adapters. Adapters have + integration tests against the real dependency. Critical CLI and daemon flows + get an end-to-end test. +- Tests cover failure paths (invalid input, missing file, timeout, dependency + error) as well as the happy path. +- Assert observable behavior: return values, emitted output, state changes. A + test that only checks a function was called, or still passes when the behavior + breaks, is not a test. +- New code ships with its tests. A change never lowers the coverage of the + packages it touches. +- A skipped test (the `skip` option in TypeScript, `t.Skip` in Go) carries the + reason and a backlog task id, in the skip message or a comment next to it. A + skip that depends on the environment (a platform, a missing build) names that + condition in its message instead. + +Correct: + +```typescript +import * as nodeTest from "node:test"; +import * as assert from "@eserstack/test-kit/assert"; + +const fakeRepo = { + findUser: (id: string) => Promise.resolve({ id, name: "Test" }), +}; + +nodeTest.test("getUser returns the stored user", async () => { + const service = createUserService(fakeRepo); + const user = await service.getUser("test-id"); + assert.assertEquals(user.name, "Test"); +}); +``` + +Incorrect: + +```typescript +// calculateDiscount ships with no test at all +export function calculateDiscount(price: number, percent: number): number { + return price * (1 - percent / 100); +} +``` + +--- + +## Runner, Files and Coverage + +Scope: TypeScript tests + +Rule: + +- Tests run on `node:test`. Assertions and test doubles come from + `@eserstack/test-kit`, imported as namespaces. Never add a third-party runner + or assertion library (vitest, jest, chai). +- Test files are co-located with the source as `*.test.ts`. +- Run tests through this codebase's test command (the manifest script its + AGENTS.md names), with coverage on, and as part of the precommit gate. CI + uploads the LCOV report so a drop is visible in review. + +`node --test` runs each test file in its own process, several files at a time, +and the tests of one file in order. A process-wide value (an environment +variable, the working directory) set in one file never reaches another, but it +stays set for every later test in the same file. Pass the value as a parameter +instead, or restore it in a `finally`. + +**Why:** `node:test` ships with Node, so the tests need no runner dependency, +and the test-kit seam lets the assertion library change in one package +(ADR 0005). + +--- + +## Table-Driven Tests + +Scope: Unit tests with several input cases + +Rule: Put the cases in a table and loop over it. Each case has a name that +describes the behavior, not the implementation. The Go form (subtests with +`t.Run`) is in go-practices. + +Correct: + +```typescript +import * as nodeTest from "node:test"; +import * as assert from "@eserstack/test-kit/assert"; + +const cases = [ + { name: "handles zero", input: 0, expected: "zero" }, + { name: "handles negative numbers", input: -1, expected: "negative" }, + { name: "handles positive numbers", input: 42, expected: "positive" }, +]; + +for (const { name, input, expected } of cases) { + nodeTest.test(name, () => { + assert.assertEquals(classifyNumber(input), expected); + }); +} +``` + +Incorrect: + +```typescript +nodeTest.test("test1", () => assert.assertEquals(classifyNumber(0), "zero")); +nodeTest.test("test2", () => + assert.assertEquals(classifyNumber(-1), "negative"), +); +``` diff --git a/.agents/skills/coding-practices/SKILL.md b/.agents/skills/coding-practices/SKILL.md new file mode 100644 index 000000000..e4a3f6540 --- /dev/null +++ b/.agents/skills/coding-practices/SKILL.md @@ -0,0 +1,39 @@ +--- +name: coding-practices +description: "Language-neutral code quality rules of this codebase for TypeScript and Go: errors, logging, explicit checks, naming and comments, timeouts and resource bounds, streaming, background work, input and config sources, performance claims. Use when writing or reviewing code, handling errors, adding logs or calling external services. Not for syntax (use javascript-practices or go-practices)." +--- + +# Coding Practices + +Rules that hold in every language in this codebase. Language syntax lives in +javascript-practices and go-practices; design patterns in design-principles. + +## Always + +- Handle every failure that can happen; catch narrowly and let the rest + propagate. No handling for states the types rule out. +- An error is handled or returned, never both: return with context, log once + where it is handled. +- Messages name the operation, the value that failed and the fix. "failed", + "invalid" and "Something went wrong" are defects. +- Log through a structured logger (`@eserstack/logging`, Go `logfx`) with ids + attached, never `console.*` (lint: `no-console`). +- Implicit truthy/falsy checks only on booleans; `??` for defaults, never `||`. +- Compare entities by id, never by slug, name or email. +- Every external call has a timeout, every pool, queue and cache a limit, and + every started promise or goroutine an owner. +- Release resources in the scope that acquired them (`defer`, `finally`). +- No absolute paths or machine-specific values; paths come from + `import.meta.dirname` or a caller-supplied root. +- Configuration precedence: environment, then config file, then default. +- Performance changes come with before and after measurements. + +## References + +| File | Read when | +| ------------------------------------------------------- | ---------------------------------------------------------------------------------------- | +| [errors.md](references/errors.md) | Raising, wrapping, matching or reporting errors; `Result` types | +| [logging.md](references/logging.md) | Adding or reviewing log calls, levels and context fields | +| [readability.md](references/readability.md) | Naming, comments, abstraction, checks, switches, early returns, line breaks | +| [resources-and-io.md](references/resources-and-io.md) | Network calls, subprocesses, retries, limits, cleanup, streaming, async work, benchmarks | +| [inputs-and-config.md](references/inputs-and-config.md) | Parsing input, resolving paths, loading configuration | diff --git a/.agents/skills/coding-practices/evals/triggers.json b/.agents/skills/coding-practices/evals/triggers.json new file mode 100644 index 000000000..0eddc4d56 --- /dev/null +++ b/.agents/skills/coding-practices/evals/triggers.json @@ -0,0 +1,66 @@ +[ + { + "query": "this catch block swallows the error, fix it", + "should_trigger": true + }, + { + "query": "improve the error message when the config file is missing", + "should_trigger": true + }, + { + "query": "add logging to the sync job", + "should_trigger": true + }, + { + "query": "the external API call has no timeout", + "should_trigger": true + }, + { + "query": "config should come from env, then file, then default", + "should_trigger": true + }, + { + "query": "this queue grows forever under load", + "should_trigger": true + }, + { + "query": "hata mesajları kullanıcıya ne yapacağını söylemiyor", + "should_trigger": true + }, + { + "query": "clean up these magic numbers and the nested ifs", + "should_trigger": true + }, + { + "query": "how do I use errors.AsType in Go 1.26", + "should_trigger": false + }, + { + "query": "which import style for @std/path", + "should_trigger": false + }, + { + "query": "tag the release", + "should_trigger": false + }, + { + "query": "add a Kubernetes deployment", + "should_trigger": false + }, + { + "query": "write the README usage example", + "should_trigger": false + }, + { + "query": "validate the webhook URL against SSRF", + "should_trigger": false + }, + { + "query": "what's our branching strategy", + "should_trigger": false + }, + { + "query": "create a new skill for UI rules", + "should_trigger": false + } +] diff --git a/.agents/skills/coding-practices/references/errors.md b/.agents/skills/coding-practices/references/errors.md new file mode 100644 index 000000000..ff65dcbf8 --- /dev/null +++ b/.agents/skills/coding-practices/references/errors.md @@ -0,0 +1,258 @@ +# Errors + +How code raises, wraps, reports and matches errors, in TypeScript and Go. Go +idioms (`%w`, `errors.Is`, `errors.AsType`) are in go-practices: Error Handling; +user-facing sanitization is in security-practices: Error Sanitization. + +## Contents + +- Error Handling +- Error Wrapping +- Actionable Error Messages +- Ignored Errors +- Handle Each Error Once +- Result Objects + +--- + +## Error Handling + +Scope: All code, in TypeScript and Go + +Rule: Handle every failure that can actually happen: I/O, network, parsing, user +input, a dependency returning an error. Do not add handling for states the types +or an invariant already rule out; a check for the impossible hides the real +contract and never runs in a test. Catch narrowly: handle the error types you +expect and let the rest propagate. A normal outcome (a missing optional file, a +404 for a lookup) is a return value, not an exception. + +Correct: + +```typescript +export async function fetchManifest( + url: string, + signal: AbortSignal, +): Promise { + const response = await fetch(url, { signal }); + if (response.status === 404) { + return null; // no manifest is a normal answer for this registry + } + if (!response.ok) { + throw new ManifestFetchError(url, response.status); + } + return (await response.json()) as Manifest; +} +``` + +Incorrect: + +```typescript +try { + return await fetchManifest(url, signal); +} catch { + return null; // an outage, a bad URL and a bug all look like "no manifest" +} +``` + +**Why:** a catch-all turns every failure into the same silent answer, and the +caller cannot tell a missing value from a broken system. + +--- + +## Error Wrapping + +Scope: Errors raised or propagated in TypeScript + +Rule: Throw `Error` subclasses named for the domain failure (`ValidationError`, +`ManifestFetchError`), with the identifying data as properties. When +propagating, wrap with `{ cause: error }` and a message naming the step that +failed, so the chain reads from the outermost operation to the root cause. Never +throw strings, never rethrow a new error that copies only `.message`, and never +build messages by string concatenation. + +Correct: + +```typescript +export class ManifestFetchError extends Error { + constructor( + readonly url: string, + readonly status: number, + options?: ErrorOptions, + ) { + super(`fetch manifest ${url}: HTTP ${status}`, options); + this.name = "ManifestFetchError"; + } +} + +try { + await installPackage(spec); +} catch (error) { + throw new InstallError(`install ${spec.name}@${spec.version}`, { + cause: error, + }); +} +``` + +Incorrect: + +```typescript +throw "install failed"; // not an Error, no stack (lint: no-throw-literal) +throw new Error(error.message); // loses the stack and the cause chain +throw new Error("Something went wrong"); // nothing to act on +``` + +In Go, the same rule is `fmt.Errorf("step: %w", err)` with sentinel errors and +custom types; see go-practices: Error Handling. + +**Why:** the type lets callers match without parsing text, and the cause chain +keeps the root error that a copied message throws away. + +--- + +## Actionable Error Messages + +Scope: Every error a user or developer can see: thrown errors, CLI output, HTTP +and daemon error responses + +Rule: An error message states what went wrong, why, and what to do next. Include +the value, path or identifier that failed. A validation error says what a valid +value looks like. "Something went wrong", "failed" and a bare "invalid" are +defects. Messages name the operation ("install foo@1.2.0: ...") instead of +starting with "Failed to". + +Correct: + +```typescript +throw new ValidationError( + `email "${email}" has no "@"; expected an address like name@example.com`, + "email", +); +``` + +Incorrect: + +```typescript +throw new Error("Invalid email"); +``` + +**Why:** the person reading the message usually has only the message; one that +names the value and the fix saves a trip into the code. + +--- + +## Ignored Errors + +Scope: All languages + +Rule: Never ignore returned errors or rejected promises. An empty catch is +allowed only on a cleanup path where a second failure must not hide the first, +and it carries a comment saying why. + +Correct: + +```typescript +try { + await runtime.fs.remove(tempDir, { recursive: true }); +} catch { + // Best-effort cleanup after the real error was already reported. +} +``` + +Incorrect: + +```typescript +try { + await saveSession(session); +} catch { + // ignored +} +``` + +```go +result, _ := operation() // error discarded +``` + +**Why:** an ignored error surfaces later as corrupted state, far from the call +that could have explained it. + +--- + +## Handle Each Error Once + +Scope: All error handling, in TypeScript and Go + +Rule: An error is either handled or returned, never both. Code that returns an +error adds context and passes it up without logging it. The code that handles it +(a request handler, a CLI command, a worker loop, a cleanup path that decides to +continue) logs it once with full context and turns it into a response, a retry +or a fallback. + +Correct: + +```go +func (s *SessionService) Load(ctx context.Context, id string) (*Session, error) { + session, err := s.store.Get(ctx, id) + if err != nil { + return nil, fmt.Errorf("load session %q: %w", id, err) // returned, not logged + } + return session, nil +} + +func (h *Handler) GetSession(w http.ResponseWriter, r *http.Request) { + session, err := h.sessions.Load(r.Context(), r.PathValue("id")) + if err != nil { + h.logger.ErrorContext(r.Context(), "get session", "error", err) // handled here + writeError(w, err) + return + } + writeJSON(w, session) +} +``` + +Incorrect: + +```go +if err != nil { + log.Error("store get failed", "error", err) + return nil, err // logged here and again by every caller +} +``` + +**Why:** logging and then returning writes the same failure several times, each +entry with less context than the last. + +--- + +## Result Objects + +Scope: TypeScript functions whose failure is an expected outcome the caller must +branch on (parsing, validation, lookups) + +Rule: Use the `Result` type from `@eserstack/primitives/results` (`ok`, `fail`, +`isOk`, `isFail`, `map`, `flatMap`) instead of a hand-rolled union. Throw for +programming errors and unexpected failures; return a `Result` when failing is +part of the function's normal contract. + +Correct: + +```typescript +import * as results from "@eserstack/primitives/results"; + +export function parsePort( + input: string, +): results.Result { + const port = Number(input); + if (!Number.isInteger(port) || port < 1 || port > 65_535) { + return results.fail( + new ValidationError( + `port "${input}" is not an integer in 1..65535`, + "port", + ), + ); + } + return results.ok(port); +} +``` + +**Why:** one shared type keeps the combinators and type guards consistent across +packages. diff --git a/.agents/skills/coding-practices/references/inputs-and-config.md b/.agents/skills/coding-practices/references/inputs-and-config.md new file mode 100644 index 000000000..2f4d70dd5 --- /dev/null +++ b/.agents/skills/coding-practices/references/inputs-and-config.md @@ -0,0 +1,151 @@ +# Inputs and Configuration + +Where values come from: validated input, configuration sources and paths, in +TypeScript and Go. Validation at trust boundaries (HTTP, CLI, files, LLM output) +is owned by security-practices: Input Validation. + +## Contents + +- Input Validation +- No Machine-Specific Assumptions +- Configuration Precedence + +--- + +## Input Validation + +Scope: Functions that receive data they did not produce + +Rule: Validate data where it enters the program, following security-practices: +Input Validation, and turn it into a typed value there. Code past that boundary +trusts the types and does not re-validate. Report a rejected value with an +actionable message (errors.md: Actionable Error Messages), naming the value and +what a valid one looks like. + +Correct: + +```typescript +export function parseSessionId(input: string): SessionId { + if (!SESSION_ID_PATTERN.test(input)) { + throw new ValidationError( + `session id "${input}" must be 1-64 letters, digits, "-" or "_"`, + "sessionId", + ); + } + return input as SessionId; +} +``` + +Incorrect: + +```typescript +export function loadSession(id: string) { + return runtime.fs.readTextFile(`${root}/${id}.json`); // "../" reaches any file +} +``` + +**Why:** one check at the boundary is easy to audit; checks scattered through +the code get skipped on some path. + +--- + +## No Machine-Specific Assumptions + +Scope: Source, scripts, tests and config committed to this codebase + +Rule: No absolute paths, user names, home directories or local ports from one +machine. Read machine settings from environment variables with defaults, and +derive every path from one of two anchors: + +- Files that ship with the package (templates, defaults, fixtures) resolve + relative to the module itself: `import.meta.dirname` in TypeScript, `embed` or + the package directory in Go. +- Files that belong to the user's project resolve against a root the caller + passes in (a `projectRoot` or `root` parameter) that defaults to the working + directory. + +Correct: + +```typescript +import { runtime } from "@eserstack/standards/cross-runtime"; + +const templateDir = runtime.path.join(import.meta.dirname, "templates", name); + +export async function analyzeComponents( + srcDir: string, + options: { projectRoot?: string } = {}, +): Promise { + const root = options.projectRoot ?? runtime.process.cwd(); + // ... +} +``` + +```go +func LoadManifest(root string) (*Manifest, error) { + return readManifest(filepath.Join(root, ".eser", "manifest.yml")) +} +``` + +Incorrect: + +```typescript +const root = "/Users/me/projects/app"; // one machine only +// process.cwd() is a hidden global, so a test cannot point this elsewhere +const config = runtime.path.join(process.cwd(), "config.ts"); +``` + +**Why:** taking the root as a parameter lets a test point the function at a +temporary directory without changing global state, and the code runs on any +machine and in CI. + +--- + +## Configuration Precedence + +Scope: Loading configuration in any package or binary + +Rule: Resolve each setting in this order, highest first: environment variable, +config file, built-in default. Start from the defaults, apply the file, then +apply the environment. A failure to read or parse a config file that exists is +an error, not a silent fall back to defaults. Settings that decide where +credentials are sent or stored come from the process environment only, never +from a file in the working directory (security-practices: Secrets Come From the +Environment). + +Correct: + +```go +func LoadConfig(root string) (Config, error) { + cfg := defaultConfig() + + data, err := os.ReadFile(filepath.Join(root, "config.json")) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return cfg, fmt.Errorf("read config: %w", err) + } + if err == nil { + if err := json.Unmarshal(data, &cfg); err != nil { + return cfg, fmt.Errorf("parse config.json: %w", err) + } + } + + if port := os.Getenv("API_PORT"); port != "" { + cfg.Port = port // environment wins + } + return cfg, nil +} +``` + +Incorrect: + +```go +file, _ := os.Open("config.json") +json.NewDecoder(file).Decode(&cfg) // error ignored, file never closed +``` + +In Go services, ajan's `configfx` (`github.com/eser/stack/pkg/ajan/configfx`) +loads files, env files and the system environment in this order into structs +tagged with `conf:"..."`; use it instead of hand-written loading. + +**Why:** a fixed order lets an operator override any setting without editing +files, and a silently ignored broken file runs the program with settings nobody +chose. diff --git a/.agents/skills/coding-practices/references/logging.md b/.agents/skills/coding-practices/references/logging.md new file mode 100644 index 000000000..6fc712c86 --- /dev/null +++ b/.agents/skills/coding-practices/references/logging.md @@ -0,0 +1,58 @@ +# Logging + +Where and how code logs, in TypeScript (`@eserstack/logging`) and Go (ajan's +`logfx`, `github.com/eser/stack/pkg/ajan/logfx`). Secrets and personal data +rules are in security-practices: Secrets and Personal Data Never Reach Output. + +--- + +## Structured Logging + +Scope: Services, adapters and command handlers (daemons, HTTP and CLI handlers, +repository adapters). Pure domain functions and library utilities do not log; +they return values or errors, and the caller logs. + +Rule: Log through this codebase's loggers with structured properties, never with +`console.*` (lint: `no-console`) or `fmt.Println`. Pick the level by layer and +attach correlation context. + +- Repository and data layer: `debug` and `trace`, `warn` for recoverable + anomalies. +- Service layer: `info` for completed operations that matter to an operator. +- Handlers: `error` once, where the failure is handled (errors.md: Handle Each + Error Once). +- Every entry carries the ids needed to follow one operation: trace or request + id, plus the entity id. Attach them once with `logger.with(...)` or + `logging.context.withContext(...)`. Extra arguments to `info(...)` land in a + single `args` attribute, so put fields in `with(...)`, not after the message. + +Correct: + +```typescript +import * as logging from "@eserstack/logging"; + +const logger = logging.logger.getLogger(["app", "sessions"]); + +export async function createSession(input: SessionInput, requestId: string) { + const log = logger.with({ requestId, projectId: input.projectId }); + const session = await store.insert(input); + await log.with({ sessionId: session.id }).info("session created"); + return session; +} +``` + +```go +logger.InfoContext(ctx, "session created", "sessionId", session.ID) +logger.DebugContext(ctx, "fetching session from store", "sessionId", id) +``` + +Incorrect: + +```typescript +console.log("session created"); // no structure, no context, fails lint +await logger.info("db query executed"); // wrong level for the repository layer +await logger.error("error"); // no context +``` + +**Why:** an operator diagnosing a failure has only the log; entries without +level discipline and ids cannot be filtered or joined into one story. diff --git a/.agents/skills/coding-practices/references/readability.md b/.agents/skills/coding-practices/references/readability.md new file mode 100644 index 000000000..38fbcb27a --- /dev/null +++ b/.agents/skills/coding-practices/references/readability.md @@ -0,0 +1,393 @@ +# Readability + +Naming, comments, abstraction, checks and control flow, in TypeScript and Go. +Formatting itself belongs to oxfmt and `gofmt`; what they enforce is not +repeated here. + +## Contents + +- Self-Documenting Code +- Comments +- Don't Repeat Yourself +- Magic Values +- Explicit Checks +- Exhaustive Branching +- Identity Comparison +- Early Returns +- Line Length and Breaking +- Prefix Extraction +- No Runtime Code Generation or Global Patching +- Circular Dependencies + +--- + +## Self-Documenting Code + +Scope: All languages + +Rule: Name similar operations the same way within a package and across packages: +all loaders are `load*`, not a mix of `load*`, `fetch*` and `get*`. Use full +words, not abbreviations (`sessionRegistry`, not `sessReg`). Casing follows the +language. In TypeScript and JavaScript, `validate-naming-and-todos` rejects a +declared name that is not camelCase, PascalCase or SCREAMING_SNAKE_CASE, since +oxlint has no casing rule. + +**Why:** consistent verbs let a reader and a search find every operation of one +kind; a mix hides some of them. + +--- + +## Comments + +Scope: All languages + +Rule: Comments explain why: a trade-off, a constraint, a non-obvious algorithm. +Every deliberate shortcut or exception (a lint ignore, a validator exclude, a +hardcoded limit, a skipped case) carries a comment giving the reason and what +would remove it. Do not commit commented-out code; git history keeps it. A TODO +names its backlog task, `TODO(TASK-12): ...` (workflow-practices: Complete, +Honest Work). `validate-naming-and-todos` rejects a TODO or FIXME comment +without a `(TASK-)` tag; text inside string and template literals is not +read. A lint exception uses oxlint's pragma with the rule name, +`// oxlint-disable-next-line /`, or `//nolint:` in Go. + +Correct: + +```typescript +// The FFI bridge returns untyped JSON; the schema check below narrows it. +// Remove the ignore when the bridge emits typed results. +// oxlint-disable-next-line typescript/no-explicit-any +const raw: any = bridge.call("status"); +``` + +Incorrect: + +```typescript +// oxlint-disable-next-line typescript/no-explicit-any +const raw: any = bridge.call("status"); +``` + +**Why:** an unexplained exception looks like an oversight, so the next person +either copies it or removes it without knowing what breaks. + +--- + +## Don't Repeat Yourself + +Scope: All languages + +Rule: Extract repeated logic at its third real use, or at the second when the +logic is complex or must stay in sync (a validation rule, a path layout, a wire +format). Lines that only look alike but encode different decisions are not +repetition; keep them apart. Prefer a plain function over a configurable +abstraction; add parameters when a caller needs them, not in advance. + +**Why:** duplicated logic drifts apart when one copy is fixed; a premature +abstraction couples unrelated callers and is harder to undo than a copy. + +--- + +## Magic Values + +Scope: All languages + +Rule: A number or string with meaning beyond its value (a limit, a timeout, a +status name, a path segment) is a named constant, and its unit is in the name +(`API_TIMEOUT_MS`, `maxPooledBuffer`). The comment on it says why that value. + +**Why:** the name says what the value means and gives one place to change it. + +--- + +## Explicit Checks + +Scope: TypeScript and JavaScript + +Rule: Use implicit truthy/falsy checks only on booleans. Compare everything else +explicitly: `=== null`, `=== undefined`, `=== ""`, `.length === 0`, `=== 0`, +`!== -1`. Defaults use `??`, never `||`. Equality is always `===` (lint: +`eqeqeq`). + +Correct: + +```typescript +if (value === null) { +} +if (name === "") { +} +if (items.length === 0) { +} +if (index !== -1) { +} +if (!response.ok) { +} // boolean +const port = config.port ?? 8000; +``` + +Incorrect: + +```typescript +if (!value) { +} // also true for 0, "" and false +if (!items.length) { +} +if (user) { +} +const port = config.port || 8000; // replaces a valid 0 +``` + +**Why:** `0`, `""` and `false` are often valid values; an implicit check treats +them as missing and the bug shows up only with those inputs. + +--- + +## Exhaustive Branching + +Scope: `switch` or `if` chains over unions and enums, in TypeScript and Go + +Rule: Handle every case. In TypeScript, end the switch with a `never` check so a +new member fails compilation. In Go, a `switch` over an enum-like type has a +`default` that returns an error. + +Correct: + +```typescript +switch (state.kind) { + case "idle": + return renderIdle(); + case "running": + return renderRunning(state); + default: { + const unreachable: never = state; + throw new Error(`unhandled state ${JSON.stringify(unreachable)}`); + } +} +``` + +```go +default: + return fmt.Errorf("%w: %q", ErrUnknownPhase, phase) +``` + +Incorrect: + +```typescript +switch (state.kind) { + case "idle": + return renderIdle(); +} // "running" silently returns undefined +``` + +**Why:** the compiler then finds every switch a new state must be added to. + +--- + +## Identity Comparison + +Scope: All languages + +Rule: Compare entities by their stable ids, never by display strings such as +slugs, usernames, emails or titles. + +Correct: + +```typescript +const isOwnProfile = membership.profileId === profile.id; +``` + +Incorrect: + +```typescript +const isOwnProfile = membership.profileSlug === profile.slug; // renames break it +``` + +**Why:** display strings change and differ in case and whitespace; ids do not. + +--- + +## Early Returns + +Scope: All languages + +Rule: Handle errors and edge cases first and return early, so the happy path +stays at the lowest indentation and reads straight down. Each guard checks one +condition and leaves; the main work comes after all of them, not in an `else`. + +Correct: + +```go +func LoadSession(root, sessionID string) (*Session, error) { + if !ValidSessionID(sessionID) { + return nil, fmt.Errorf("load session %q: %w", sessionID, ErrInvalidSessionID) + } + + data, err := os.ReadFile(filepath.Join(root, sessionID+".json")) + if err != nil { + return nil, fmt.Errorf("read session %q: %w", sessionID, err) + } + + return decodeSession(data) +} +``` + +Incorrect: + +```typescript +function sessionStatus(session: Session | null): Status { + if (session !== null) { + if (session.active) { + return loadStatus(session.id); + } else { + return "inactive"; + } + } else { + return "missing"; + } +} +``` + +**Why:** nested branches make the reader hold every condition at once; guards +retire them one by one. + +--- + +## Line Length and Breaking + +Scope: All code; TypeScript and JavaScript are wrapped by oxfmt, so this governs +Go, Markdown and anything else the formatter does not wrap + +Rule: There is no fixed column limit, but a line past about 120 characters is +broken. Break where the structure already has a seam (between arguments, after +an operator, before a chained call), not at whatever column the line reaches. A +call with four or more arguments puts each argument on its own line, with the +closing parenthesis on its own line. This holds for code written on request too; +asking for a one-liner does not make a long call acceptable. Where oxfmt owns +the layout, keep its output rather than hand-wrapping against it. + +A signature that is too long to read usually has too many parameters. The fix is +an options struct or object (see architecture-guidelines: Public API and CLI +Surface), not cleverer wrapping. + +Correct: + +```go +handleSessions( + w, + r, + registry, + cfg, + logger, +) + +type StartOptions struct { + Root string + Timeout time.Duration + Logger *logfx.Logger +} + +func StartSession(ctx context.Context, opts StartOptions) (*Session, error) +``` + +Incorrect: + +```go +mux.HandleFunc("/api/sessions", func(w http.ResponseWriter, r *http.Request) { handleSessions(w, r, registry, cfg, logger) }) + +func StartSession(ctx context.Context, root string, timeout time.Duration, logger *logfx.Logger, retries int, dryRun bool) (*Session, error) +``` + +**Why:** a line broken at its seams can be read and diffed argument by argument. + +--- + +## Prefix Extraction + +Scope: Taking a value after a known prefix: HTTP headers, URIs, file paths, +protocol strings, in TypeScript and Go + +Rule: Check that the value starts with the prefix, then cut exactly the prefix +length. Never use a search-and-replace for this: it matches the first occurrence +anywhere in the string, so `"xBearer token"` or `"s3://file://bucket"` are +silently rewritten. Return "no value" when the prefix is missing or nothing +follows it. + +Correct: + +```typescript +function extractAfterPrefix( + value: string | null, + prefix: string, +): string | null { + if (value === null || !value.startsWith(prefix)) { + return null; + } + const extracted = value.slice(prefix.length); + return extracted.length > 0 ? extracted : null; +} + +const token = extractAfterPrefix(authHeader, "Bearer "); +``` + +```go +token, ok := strings.CutPrefix(authHeader, "Bearer ") +if !ok || token == "" { + return "", ErrMissingToken +} +``` + +Incorrect: + +```typescript +const token = authHeader?.replace("Bearer ", "") ?? null; // matches anywhere +const token2 = authHeader?.slice(7) ?? null; // never checks the prefix +``` + +```go +token := strings.Replace(authHeader, "Bearer ", "", 1) // matches anywhere +``` + +**Why:** a replace that matches mid-string accepts malformed input as valid, +which in a header parser is an authentication bug. + +--- + +## No Runtime Code Generation or Global Patching + +Scope: All languages + +Rule: Do not build and run code at runtime, and do not change shared built-ins +or other packages' internals. In TypeScript that means no `eval` (lint: +`no-eval`), no `new Function`, no prototype changes and no +`Object.defineProperty` on shared objects. In Go it means no `unsafe` or +`reflect` writes into another package's unexported state. + +Correct: + +```typescript +const handlers: Record Output> = { add, remove }; +const handler = handlers[action]; +``` + +Incorrect: + +```typescript +eval(`${action}(input)`); // any action string becomes code +Array.prototype.last = function () {}; // changes every array in the process +``` + +**Why:** both break static analysis, and any input that reaches them becomes +code. + +--- + +## Circular Dependencies + +Scope: Imports between TypeScript modules and packages + +Rule: Module imports form a graph without cycles. Check for cycles with a tool +(`eser codebase validate-circular-deps` from `@eserstack/codebase`) after moving +code between modules, whether or not the precommit gate runs it. Break a cycle +by moving the shared part into a module both sides import. + +**Why:** a cycle makes evaluation order decide which side sees `undefined` +exports, and the failure depends on which module is imported first. diff --git a/.agents/skills/coding-practices/references/resources-and-io.md b/.agents/skills/coding-practices/references/resources-and-io.md new file mode 100644 index 000000000..61298776b --- /dev/null +++ b/.agents/skills/coding-practices/references/resources-and-io.md @@ -0,0 +1,222 @@ +# Resources and I/O + +Timeouts, limits, cleanup, streaming, background work and performance claims, in +TypeScript and Go. Go specifics: go-practices context, data-structures and +performance references. + +## Contents + +- External Calls Have Timeouts +- Bound Every Resource +- Release Resources Where They Are Acquired +- Stream Large Data +- Owned Asynchronous Work +- Performance Claims Need Measurement + +--- + +## External Calls Have Timeouts + +Scope: Every network request, subprocess and other call that leaves the process, +in TypeScript and Go + +Rule: Every external call has a timeout, so one slow dependency cannot hang the +caller. Retry only operations that are safe to repeat (reads, idempotent +writes), with exponential backoff and a maximum number of attempts, and never +retry a validation or authorization failure. Between attempts, check whether the +caller has cancelled (Go `ctx.Err()`, TypeScript `signal.aborted`) and wait with +a timer that cancellation interrupts, so a retry loop never outlives its caller. +A caller that makes many calls to the same dependency bounds its concurrency. + +Correct: + +```typescript +import * as shellExec from "@eserstack/shell/exec"; + +const response = await fetch(url, { + signal: AbortSignal.any([signal, AbortSignal.timeout(10_000)]), +}); +await shellExec.exec`git fetch origin`.timeout(60_000).text(); +``` + +```go +client := httpclient.NewClient(httpclient.WithConfig(cfg)) // ajan httpclient: timeouts and retries from config +``` + +Incorrect: + +```typescript +const response = await fetch(url); // waits forever on a stalled server +``` + +**Why:** without a bound, the slowest dependency sets the latency of everything +that calls it, and a stuck call holds its connection and memory indefinitely. + +--- + +## Bound Every Resource + +Scope: Pools, queues, buffers, caches, concurrency, request and file sizes, in +TypeScript and Go + +Rule: Everything that can grow has a limit: connection and worker pools, queue +and channel depths, in-memory caches, concurrent goroutines or promises, and the +size of what is read from a request, a file or a subprocess. Pick the limit on +purpose, make it a setting with a safe default when callers may need another +value, and decide what happens at the limit: wait, reject with an error, or drop +the oldest entry. + +Correct: + +```go +jobs := make(chan Job, 100) // producers block when 100 jobs are waiting +g.SetLimit(4) // at most 4 concurrent syncs +body := http.MaxBytesReader(w, r.Body, 1<<20) +``` + +Incorrect: + +```go +for _, p := range projects { + go sync(p) // one goroutine per project, however many there are +} +``` + +**Why:** an unbounded resource grows until the process runs out of memory or +file handles, usually under the load where it matters most. + +--- + +## Release Resources Where They Are Acquired + +Scope: Files, sockets, locks, timers, subprocesses, in TypeScript and Go + +Rule: Release a resource in the same scope that acquired it, set up right after +the acquisition succeeds: `defer` in Go, `try`/`finally` or `await using` in +TypeScript. A failure to close a writer can mean lost data, so report it. + +Correct: + +```typescript +const lock = await acquireLock(lockPath); +try { + return await updateState(state); +} finally { + await lock.release(); +} +``` + +Incorrect: + +```typescript +const lock = await acquireLock(lockPath); +const result = await updateState(state); // a throw here keeps the lock forever +await lock.release(); +``` + +**Why:** cleanup at the end of a function is skipped by the first early return +or thrown error someone adds later. + +--- + +## Stream Large Data + +Scope: Reading, transforming or transferring data whose size is not small and +fixed, in TypeScript and Go + +Rule: Process large or unbounded data as a stream: rows one at a time, files and +responses through streams, results through lazy iterators. Load all at once only +when the size is small and known. + +Correct: + +```typescript +const response = await fetch(logUrl, { signal }); +if (response.body === null) { + throw new Error(`fetch ${logUrl}: response has no body`); +} +// readLines: an async generator over a byte stream +for await (const line of readLines(response.body)) { + handle(line); +} +``` + +Incorrect: + +```typescript +const text = await runtime.fs.readTextFile(logPath); // the whole log in memory +text.split("\n").forEach(handle); +``` + +**Why:** memory stays constant however large the input grows, and the first +result arrives before the last byte is read. + +--- + +## Owned Asynchronous Work + +Scope: Promises in TypeScript, goroutines in Go + +Rule: Every asynchronous operation that is started is either awaited or owned by +something that waits for it and handles its error. Work that is deliberately +detached goes through a named helper that logs or reports failures. + +Correct: + +```typescript +await Promise.all(files.map((f) => processFile(f))); + +runInBackground("refresh cache", () => refreshCache()); // helper catches + logs +``` + +```go +g, ctx := errgroup.WithContext(ctx) +for _, f := range files { + g.Go(func() error { return processFile(ctx, f) }) +} +if err := g.Wait(); err != nil { + return fmt.Errorf("process files: %w", err) +} +``` + +Incorrect: + +```typescript +files.forEach((f) => processFile(f)); // rejections are lost +``` + +```go +go processFile(ctx, f) // nobody waits, the error is dropped +``` + +**Why:** an unowned promise loses its rejection; an unowned goroutine leaks and +hides its failure. + +--- + +## Performance Claims Need Measurement + +Scope: Changes made for performance, and any code or review comment claiming one + +Rule: A performance claim comes with a measurement: a mitata `*.bench.ts` run +with `node`, or `go test -bench`, before and after, on the same machine. Report +the numbers and the command. Without a measurement, write the plain version. + +- Profile before choosing what to change. If most of the time is spent waiting + on a database, an upstream service or the disk, fix that component; faster + code in the process will not show. +- Fix the algorithm before the constants: replacing a linear search in a loop + with a map lookup beats any tuning of the loop. +- Change one thing at a time and repeat each run several times; a single run + before and after cannot tell a small gain from noise. +- Comment an optimization that makes the code less obvious with what it avoids + and the measured gain, so nobody reverts it as needless complexity. +- Check new loops over I/O for per-item calls that can be batched (lint: + `no-await-in-loop` flags the TypeScript case), and check that buffers and + caches have a bound. + +The Go procedure (`benchstat`, pprof) is in go-practices: Benchmark, Change, +Compare. + +**Why:** intuition about bottlenecks is usually wrong, and an unmeasured +optimization costs readability for a gain nobody can confirm. diff --git a/.agents/skills/design-principles/SKILL.md b/.agents/skills/design-principles/SKILL.md new file mode 100644 index 000000000..42acfdfb2 --- /dev/null +++ b/.agents/skills/design-principles/SKILL.md @@ -0,0 +1,32 @@ +--- +name: design-principles +description: "How this codebase designs functions and modules in TypeScript and Go: pure functions with thin effectful wrappers, readonly data, small injected interfaces, plain data objects, one clock read, cancellation handles, collections by access pattern, side-effect-free modules. Use when shaping signatures, modules, state or collections. Not for system layout (use architecture-guidelines)." +--- + +# Design Principles + +How functions, modules, data and state are shaped in this codebase. System +structure lives in architecture-guidelines; code-level quality rules in +coding-practices. + +## Always + +- Logic lives in pure functions with explicit parameters; a thin wrapper reads + configuration and performs the effect. +- Return new values instead of mutating inputs; mark unmutated parameters and + fields `readonly`. +- Accept small interfaces and pass dependencies in; no inheritance for reuse. +- Data is plain objects plus functions; classes only for stateful services, no + getters or setters. +- Read the clock once per operation and pass the value down. +- One cancellation handle (`ctx`, `AbortSignal`) flows through the whole + operation; never start a fresh one mid-chain. +- Pick maps and sets for lookups, arrays and slices for sequences; hand out + copies of internal collections. +- Importing a module does no I/O and reads no environment; compute lazily. + +## References + +| File | Read when | +| ----------------------------------------- | -------------------------------------------------------------------------- | +| [principles.md](references/principles.md) | Designing a function, module, type or collection; reviewing state handling | diff --git a/.agents/skills/design-principles/evals/triggers.json b/.agents/skills/design-principles/evals/triggers.json new file mode 100644 index 000000000..2f1990e26 --- /dev/null +++ b/.agents/skills/design-principles/evals/triggers.json @@ -0,0 +1,66 @@ +[ + { + "query": "this function reads the clock three times, is that a problem", + "should_trigger": true + }, + { + "query": "make this config object immutable", + "should_trigger": true + }, + { + "query": "should this module have side effects at import", + "should_trigger": true + }, + { + "query": "split this function so the logic is testable without the filesystem", + "should_trigger": true + }, + { + "query": "how should cancellation be passed to this long-running task in TS", + "should_trigger": true + }, + { + "query": "which data structure for looking up sessions by id", + "should_trigger": true + }, + { + "query": "bu interface çok büyük, nasıl küçültürüz", + "should_trigger": true + }, + { + "query": "replace this class with getters and setters by something simpler", + "should_trigger": true + }, + { + "query": "add a GitHub Actions job", + "should_trigger": false + }, + { + "query": "fix the failing release run", + "should_trigger": false + }, + { + "query": "which package registry do we prefer", + "should_trigger": false + }, + { + "query": "write the ADR", + "should_trigger": false + }, + { + "query": "check if CONTRIBUTING.md is up to date", + "should_trigger": false + }, + { + "query": "add a new package to the monorepo", + "should_trigger": false + }, + { + "query": "the JWT secret is hardcoded", + "should_trigger": false + }, + { + "query": "ask me what you need before you start", + "should_trigger": false + } +] diff --git a/.agents/skills/design-principles/references/principles.md b/.agents/skills/design-principles/references/principles.md new file mode 100644 index 000000000..2134b2c01 --- /dev/null +++ b/.agents/skills/design-principles/references/principles.md @@ -0,0 +1,345 @@ +# Design Principles + +How functions, modules, data and state are shaped, in TypeScript and Go. +Architecture-level structure (ports, adapters, composition roots) is in +architecture-guidelines: Hexagonal Architecture (Double-Layered). + +## Contents + +- Pure Functions +- Immutability +- Small Interfaces +- Data as Plain Objects +- One Clock Read per Operation +- Cancellation Flows With the Work +- Choose Data Structures by Access Pattern +- Module-Level Side Effects + +--- + +## Pure Functions + +Scope: All languages + +Rule: Put logic in pure functions that take everything they need as parameters +and return a result without touching outside state. Keep pure logic in modules +that do not import environment or framework state (env variables, build-tool +globals, UI context, process-wide singletons). When a module needs both, put the +logic in its own file with explicit parameters and let a thin wrapper inject the +configuration. A function that must have an effect keeps it at the edge: compute +first, then perform one effect with the result. + +Correct: + +```typescript +// discount.ts: pure, testable without setup +export function applyDiscount(order: Order, rate: number): Order { + return { ...order, total: order.total * (1 - rate) }; +} + +// checkout.ts: thin wrapper that reads configuration and performs the effect +export async function checkout(order: Order, deps: CheckoutDeps) { + const priced = applyDiscount(order, deps.config.discountRate); + await deps.orders.save(priced); +} +``` + +Incorrect: + +```typescript +let appliedDiscount = 0; // hidden state shared by every caller + +export function applyDiscount(order: Order): Order { + appliedDiscount = order.total * runtime.env.get("DISCOUNT_RATE"); // reads env + return { ...order, total: order.total - appliedDiscount }; +} +``` + +**Why:** a pure function is tested with plain values, and a change in the +environment cannot change its result. + +--- + +## Immutability + +Scope: All languages + +Rule: Do not mutate inputs or shared data; return a new value. In TypeScript, +mark properties and array or object parameters that are not mutated as readonly +(`readonly T[]`, `Readonly`, `as const` for literal tables), so the compiler +enforces it. Mutation of a local value that nobody else can see is fine. + +Correct: + +```typescript +export function addTag(session: Readonly, tag: string): Session { + return { ...session, tags: [...session.tags, tag] }; +} +``` + +Incorrect: + +```typescript +export function addTag(session: Session, tag: string): Session { + session.tags.push(tag); // the caller's object changes too + return session; +} +``` + +**Why:** a caller that passed a value does not expect it to change; readonly +types turn that expectation into a compile error. + +--- + +## Small Interfaces + +Scope: Interfaces and the functions that accept them, in TypeScript and Go + +Rule: Define small, purpose-specific interfaces in the package that uses them +(in a hexagonal package, the `domain/` that declares the port). Public functions +accept an interface, not a concrete type, so callers can pass any implementation +and tests can pass a fake. Pass dependencies in (parameters, a deps object, a +constructor) instead of inheriting behavior from a base class. Do not add a +wrapper that only forwards to another method; call the method directly. + +Correct: + +```typescript +type Clock = { now(): number }; + +export function isExpired(session: Session, clock: Clock): boolean { + return session.expiresAt <= clock.now(); +} +``` + +```go +type SessionReader interface { + Get(ctx context.Context, id string) (*Session, error) +} + +func Summarize(ctx context.Context, r SessionReader, id string) (Summary, error) +``` + +Incorrect: + +```go +type EverythingStore interface { + Read() error + Write() error + Delete() error + Update() error + Migrate() error // every caller depends on all of it +} +``` + +**Why:** a small interface is cheap to fake in a test and leaves each caller +coupled only to what it uses. + +--- + +## Data as Plain Objects + +Scope: TypeScript types and classes + +Rule: Model data as plain objects with `readonly` fields and put behavior in +functions that take them. Use a class only for a stateful service that owns +resources or a cache. Do not use getters or setters: a property access that runs +code (logging, validation, lazy loading) surprises the caller. An update is a +function that returns a new object. + +Correct: + +```typescript +type User = { readonly id: string; readonly email: string }; + +export function changeEmail(user: User, email: string): User { + return { ...user, email }; +} + +export class SessionRegistry { + readonly #sessions = new Map(); + // ... +} +``` + +Incorrect: + +```typescript +class User { + #email = ""; + get email(): string { + this.logAccess(); // a read with a side effect + return this.#email; + } +} +``` + +**Why:** plain data serializes, compares and copies without surprises; state +belongs in the few objects that manage it. + +--- + +## One Clock Read per Operation + +Scope: Code that timestamps or compares times + +Rule: Read the clock once per logical operation and pass the value down. Do not +call `new Date()`, `Date.now()` or `time.Now()` inside a loop or in several +steps that must agree on the same time. Taking the clock as a parameter also +makes the code testable. + +Correct: + +```typescript +const now = Date.now(); +const expired = sessions.filter((s) => s.expiresAt <= now); +``` + +Incorrect: + +```typescript +const expired = sessions.filter((s) => s.expiresAt <= Date.now()); +``` + +**Why:** two reads in one operation can straddle a boundary and give the steps +different answers about the same moment. + +--- + +## Cancellation Flows With the Work + +Scope: Any operation that can be abandoned: requests, CLI commands, daemon RPCs, +background jobs, in TypeScript and Go + +Rule: An operation carries one cancellation handle from the point it starts to +every call it makes: a `context.Context` in Go, an `AbortSignal` in TypeScript. +Pass the caller's handle down instead of creating a new one, so when the caller +gives up (a client disconnects, a user presses Ctrl-C, a deadline passes) +everything the operation started stops. A step may shorten the remaining time +for its own part, never extend it. Work that must finish after the caller leaves +is detached on purpose, gets its own time limit and an owner that reports its +failure. + +Correct: + +```typescript +export async function syncAll( + projects: readonly Project[], + signal: AbortSignal, +): Promise { + for (const project of projects) { + signal.throwIfAborted(); + await syncOne( + project, + AbortSignal.any([signal, AbortSignal.timeout(30_000)]), + ); + } +} +``` + +```go +func SyncAll(ctx context.Context, projects []Project) error { + for _, p := range projects { + if err := syncOne(ctx, p); err != nil { + return fmt.Errorf("sync %s: %w", p.Name, err) + } + } + return nil +} +``` + +Incorrect: + +```typescript +await syncOne(project, new AbortController().signal); // caller cannot stop it +``` + +```go +_ = syncOne(context.Background(), p) // detached from the caller's deadline +``` + +The Go form is in the go-practices context reference (`context.md`). + +**Why:** a chain that drops the handle at one link keeps consuming CPU, +connections and quota for a caller that is no longer there. + +--- + +## Choose Data Structures by Access Pattern + +Scope: Collections in TypeScript and Go + +Rule: Pick the structure from how the data is read and written: + +- Keyed lookup or membership: a map or set (`Map`/`Set` in TypeScript, `map` in + Go), not a linear search through an array. +- Ordered data read in sequence: a contiguous array or slice. Use linked + structures only for frequent insertion in the middle, and only after + measuring. +- Known or estimable size: allocate once up front instead of growing in a loop. +- Shared state handed to a caller: return a copy or a read-only view, so the + caller cannot change the owner's data. + +Correct: + +```typescript +const activeById = new Map(sessions.map((s) => [s.id, s])); +const session = activeById.get(sessionId); + +export function listIds(): readonly string[] { + return [...ids]; +} +``` + +Incorrect: + +```typescript +const session = sessions.find((s) => s.id === sessionId); // O(n) per lookup in a loop + +export function listIds(): string[] { + return ids; // caller can push into the registry's own array +} +``` + +The Go form is in the go-practices data structures reference +(`data-structures.md`). + +**Why:** the wrong structure turns a lookup loop quadratic, and a shared +internal array lets any caller corrupt the owner's state. + +--- + +## Module-Level Side Effects + +Scope: All TypeScript modules + +Rule: Importing a module runs no I/O, reads no environment, starts no timers and +does no heavy computation. Module scope holds constants, types, and function and +class declarations. State that must be computed goes behind a lazy getter that +computes on first call and caches the result (or its promise). Mutable +module-level variables exist only as that cache. Top-level `await` is banned by +lint (`no-top-level-await`). + +Correct: + +```typescript +let settingsPromise: Promise | null = null; + +export const getSettings = (): Promise => { + if (settingsPromise === null) { + settingsPromise = loadConfig().then(processConfig); + } + return settingsPromise; +}; +``` + +Incorrect: + +```typescript +const env = runtime.env.toObject(); // environment read on import +export const settings = processConfig(await loadConfig()); // I/O on import +export let currentUser: User | null = null; // global mutable state +``` + +**Why:** code at module scope runs on every import, in import order, before a +test can intercept it; one failure there breaks every importer. diff --git a/.agents/skills/documentation-conventions/SKILL.md b/.agents/skills/documentation-conventions/SKILL.md new file mode 100644 index 000000000..855072d7c --- /dev/null +++ b/.agents/skills/documentation-conventions/SKILL.md @@ -0,0 +1,39 @@ +--- +name: documentation-conventions +description: "Documentation conventions of this codebase: READMEs with namespace-import examples and emoji titles, docs that change with the code, page structure, code examples, placeholders, doc review, and plain prose without filler, unbacked claims, em dashes or AI tells. Use when writing, editing or reviewing READMEs, docs, guides, skills, ADRs, backlog items, changelogs, commit messages or code comments." +--- + +# Documentation Conventions + +Rules for what this codebase's documents say, how a page is shaped, and how its +sentences read. + +## Always + +- README examples use namespace imports of the package root + (`import * as pkg from "@scope/pkg"`), never sub-path imports; a package whose + `.` export is its CLI is imported from `/mod`. +- Never remove existing emojis (🧱 or ⚡ titles, 🔗 links footer) or descriptive + details such as "(LIFO cleanup)". +- The documentation diff ships in the same change as the code it describes; edit + JSDoc, never a generated API reference. +- No empty qualifiers (comprehensive, seamless, robust), no claim, number or + quote without a source the reader can open, no em dashes in new or edited + text. +- A docs page does one job (tutorial, how-to, reference, conceptual or + troubleshooting), opens with what it covers and for whom, and its headings say + what each section holds. +- Every code block has a language tag and prose that explains it. A value the + reader replaces is a `snake_case` placeholder such as `your_access_token`, + never `` or `xxx`. +- Reader-facing docs address the reader as "you" and give steps as imperatives. +- Before delivering, read the text as a stranger: what looks generated, and + which fact has no source? Fix both. + +## References + +| File | Read when | +| ------------------------------------------------- | -------------------------------------------------------------------------- | +| [readmes.md](references/readmes.md) | Writing a README, adding docs, changing a documented surface | +| [prose.md](references/prose.md) | Writing or reviewing any prose: docs, skills, commits, backlog, notes | +| [docs-structure.md](references/docs-structure.md) | Shaping a docs page, writing code examples or placeholders, reviewing docs | diff --git a/.agents/skills/documentation-conventions/evals/triggers.json b/.agents/skills/documentation-conventions/evals/triggers.json new file mode 100644 index 000000000..bb87a57d4 --- /dev/null +++ b/.agents/skills/documentation-conventions/evals/triggers.json @@ -0,0 +1,78 @@ +[ + { + "query": "write a README for @eserstack/cache", + "should_trigger": true + }, + { + "query": "this doc sounds generated, make it plain", + "should_trigger": true + }, + { + "query": "update the docs after changing the CLI flags", + "should_trigger": true + }, + { + "query": "write a backlog item for the flaky test", + "should_trigger": true + }, + { + "query": "the README example imports from a sub-path, fix it", + "should_trigger": true + }, + { + "query": "commit mesajını daha sade yaz", + "should_trigger": true + }, + { + "query": "keep the emojis but fix the README structure", + "should_trigger": true + }, + { + "query": "write the JSDoc for this exported function", + "should_trigger": true + }, + { + "query": "review docs/guide.md for voice and structure", + "should_trigger": true + }, + { + "query": "this tutorial mixes how-to steps and an API reference, restructure it", + "should_trigger": true + }, + { + "query": "the setup guide uses placeholders and untagged code blocks, clean it up", + "should_trigger": true + }, + { + "query": "write an ADR structure", + "should_trigger": false + }, + { + "query": "cut the release notes automatically", + "should_trigger": false + }, + { + "query": "add a CI docs job", + "should_trigger": false + }, + { + "query": "fix the Go comment lint", + "should_trigger": false + }, + { + "query": "which test runner to use", + "should_trigger": false + }, + { + "query": "add a new package", + "should_trigger": false + }, + { + "query": "clarify what the user wants", + "should_trigger": false + }, + { + "query": "security review of the README endpoint", + "should_trigger": false + } +] diff --git a/.agents/skills/documentation-conventions/references/docs-structure.md b/.agents/skills/documentation-conventions/references/docs-structure.md new file mode 100644 index 000000000..03a3715bd --- /dev/null +++ b/.agents/skills/documentation-conventions/references/docs-structure.md @@ -0,0 +1,189 @@ +# Page Structure, Examples and Review + +How a documentation page in this codebase is shaped: what job it does, how it +opens, how its headings, lists, code, placeholders and links work, and how a +page is reviewed. Adapted from vercel-labs/writing-guidelines (83e2316); the +sentence-level rules are in prose.md. + +## Contents + +- One Job per Page +- Openings, Sections and Headings +- Lists +- Code Examples +- Placeholders and Units +- Links and Terms +- Reviewing Docs + +--- + +## One Job per Page + +Scope: Guides, tutorials, docs pages and the long sections of READMEs + +Rule: Decide what kind of page you are writing before writing it, and let it do +that one job: + +| Kind | Reader | Tone | +| --------------- | ----------------------- | ---------------------------------------- | +| Tutorial | Learning by doing | Encouraging, predictable steps, no traps | +| How-to | In the middle of a task | Terse and direct | +| Reference | Looking something up | Neutral, complete, quotable | +| Conceptual | Building a mental model | Explanatory, with examples | +| Troubleshooting | Stuck on a failure | Acknowledge the problem, then fix it | + +A page that mixes a tutorial, a how-to and a reference serves none of them; +split it, or link from one to the other. The goal of the page is a verb the +reader can check ("configure retries", "debug a failed release"). The title of a +guide names the reader's task or question ("Resume a failed release"), not the +feature ("Release Resume Subsystem"). Package READMEs keep the package name as +their title (readmes.md: Package README Structure). + +When a page serves several audiences, open with a short shared part and put the +technical depth in its own sections. + +**Why:** a reader arrives with one question; a page built around a feature name +or several jobs makes them search for the part that answers it. + +--- + +## Openings, Sections and Headings + +Scope: Every documentation page + +Rule: + +- The page opens with one paragraph saying what it covers and for whom. In a + package README that is the vision paragraph under the title. +- A major section opens with a sentence saying what it covers. +- A paragraph holds one idea, in two to four sentences; split anything longer. +- Spell out an acronym on first use ("Server-Side Request Forgery (SSRF)") and + define each term the first time it appears, linking to where it is explained. +- A heading tells the reader what the section holds, so they can find a section + from the headings alone: "Retry limits when the registry is down", not "Notes" + or "Caveats". Headings fixed by a template (README "Quick Start" and "🔗 + Links", skill "Contents", "Always" and "References") stay as they are. +- Headings use Title Case, the case this codebase's READMEs, docs and skill + files already use. Skill section titles must keep their exact wording, since + other skills point to them as `skill-name: Exact Section Title`. + +**Why:** readers scan openings and headings before they read; a page whose first +paragraph and headings answer "is this the page I need" saves them the rest. + +--- + +## Lists + +Scope: Lists in any documentation + +Rule: Three or more parallel items in a sentence become a list. Introduce a list +with a colon. Use numbers for an order the reader follows (steps, lifecycles) +and bullets otherwise. Items that are full sentences end with a period; +fragments do not. + +--- + +## Code Examples + +Scope: Code blocks in READMEs, guides and docs + +Rule: + +- Every fenced block has a language tag (`typescript`, `go`, `bash`, `json`, + `yaml`, `text`), so it is highlighted and the reader knows what it is. +- Examples are written in the language of the package they document; TypeScript + for the TypeScript packages. +- Say in prose what a block does before or after it; do not drop code without + context. +- Keep a block to about 25 lines and lines to about 80 columns. Split a longer + flow into steps with prose between them. +- Leave out options set to their defaults, and do not repeat a definition an + earlier block already showed. +- Comments inside the block stay few; the explanation goes in the prose. +- Put the code in the page. "See the full example in `example.ts`" at the end of + a guide is not a substitute for the example. +- An example that names an AI model uses a current model ID, never a retired + one. + +Examples run as pasted (readmes.md: Docs Change With the Code). + +--- + +## Placeholders and Units + +Scope: Values the reader replaces, and numbers with units + +Rule: A value the reader must replace is a descriptive `snake_case` placeholder +they can double-click to select: `your_access_token`, `your_project_id`. A +placeholder number is recognizably fake: `1234567890`. Never use ``, +`xxx`, `your-token` or a bare `ABC123`, which readers paste as is. Angle +brackets stay in command synopses, where they are the usage notation +(`git switch `). + +Write sizes and durations with a space and the standard unit: `64 KB`, `5 MB`, +`200 ms`. Values written in a config or flag syntax keep that syntax (`30s`). + +Correct: + +```bash +export REGISTRY_TOKEN=your_registry_token +``` + +Incorrect: + +```bash +export REGISTRY_TOKEN= +``` + +--- + +## Links and Terms + +Scope: Links in any documentation + +Rule: Link text names the destination ("see the release process"), never "here", +"link" or a bare URL. Link a term where it is first used to the page that +explains it, rather than repeating the explanation. + +Correct: + +```markdown +Releases follow the [release process](./releasing.md). +``` + +Incorrect: + +```markdown +For releases, click [here](./releasing.md). +``` + +--- + +## Reviewing Docs + +Scope: Reviewing a page or a docs diff, yours or someone else's + +Rule: Check the page against this file, prose.md and readmes.md, then answer: + +- Can a reader with only this page complete the task it promises? +- Do the code examples run as written, and do commands and paths exist? +- Are prerequisites listed before the first step, and limits documented? +- When there are several ways to do it, does the page recommend the simplest? +- Does every heading and opening say what follows? + +Report findings grouped by file, one line each, as `file:line - issue`, with the +fix only when it is not obvious. A file with no findings gets `✓ pass`. No +preamble. + +```text +## docs/guide/releases.md + +docs/guide/releases.md:12 - title is feature-shaped, name the reader's task +docs/guide/releases.md:31 - "easy" presses the reader, describe the step +docs/guide/releases.md:58 - code block has no language tag +docs/guide/releases.md:71 - placeholder , use your_registry_token + +## README.md + +✓ pass +``` diff --git a/.agents/skills/documentation-conventions/references/prose.md b/.agents/skills/documentation-conventions/references/prose.md new file mode 100644 index 000000000..a0bb1dcd4 --- /dev/null +++ b/.agents/skills/documentation-conventions/references/prose.md @@ -0,0 +1,278 @@ +# Plain Prose + +Writing rules for every sentence a person reads in this codebase. The voice, +word and AI-tell rules include patterns adapted from +vercel-labs/writing-guidelines (83e2316). + +## Contents + +- Plain Prose +- Address the Reader +- Empty Qualifiers +- Claims Need Evidence +- No Em Dashes +- Borrowed Rhythm +- Filler and Hedging +- Name the Actor +- Neutral Terms +- Formatting That Shouts +- Audit Before Delivering + +--- + +## Plain Prose + +Scope: Every sentence a person reads: READMEs, skill docs, ADRs, backlog items, +changelog entries, commit messages, code comments + +Rule: Documentation says what the code does, in the words a maintainer would use +out loud. Text that reads as generated (empty qualifiers, borrowed rhythm, +claims with nothing behind them) is a defect even when every fact in it is true, +because readers stop trusting the page. The rules below name the patterns that +give it away. When a sentence trips one, fix the sentence; do not pad it back up +with a different pattern. + +--- + +## Address the Reader + +Scope: Text written for a reader: READMEs, guides, help text, error messages + +Rule: + +- Write to the reader as "you", never "the user" or "one can". Steps are + imperatives: "Run `pnpm install`", not "You will need to run `pnpm install`". +- "We" stands only for a deliberate decision of the maintainers ("we deprecated + the v1 API"), never for "you". +- Present tense, unless the sentence describes future behavior. +- Aim for sentences under 20 words. Read each one once at speaking pace; if it + needs a second read, name the subject, the action and the consequence, and + drop pronouns that reach back several sentences. +- No rhetorical questions; state the point. + +Rule files and skills address an agent and keep the plain imperative. + +Correct: + +```markdown +Run `pnpm install`, then start the server. +``` + +Incorrect: + +```markdown +Want to get started? First, we will need to install the dependencies. +``` + +--- + +## Empty Qualifiers + +Rule: Cut words that sound like praise and carry no information: comprehensive, +seamless, robust, powerful, elegant, cutting-edge, next-level, world-class, +effortless, streamline, leverage, harness (as a verb), unlock, elevate, empower, +delve, showcase, testament, game-changer. Replace the word with what the package +actually does, or with nothing. + +Two more groups go the same way: + +- Words that press the reader: easy, simple, simply, quick, obviously. When the + step turns out hard for this reader, the word blames them. Describe the step + instead: "one command", "the default settings", "most installs skip this". +- Filler intensifiers: very, really, just, quite. Cut them or rewrite the + sentence. + +Correct: + +```markdown +`@eserstack/parsing` is a parsing library for analyzing and tokenizing strings. + +The HTTP adapter provides HTTP connections with built-in resilience features: +``` + +Incorrect: + +```markdown +`@eserstack/parsing` is a comprehensive parsing library designed to analyze and +tokenize strings. + +The HTTP adapter provides robust, production-ready HTTP connections with +built-in resilience features: +``` + +--- + +## Claims Need Evidence + +Rule: A statement about quality, scale or adoption is either backed by something +in this codebase (a benchmark with its date and command, a test, a measured +size) or it is not written. This covers "production-ready", "battle-tested", +"blazing fast", "trusted by", "industry-leading", and any number, quote or name +that did not come from a source the reader can open. When you do not know a +fact, leave it out; never fill the gap with a plausible guess. + +The same applies to weasel words and vague quantifiers: significantly, many, +often, typically, generally, near-zero, sub-second, most requests. Give the +number and where it comes from, or name the specific case. + +Correct: + +```markdown +Measured on 2026-09-17 over the repository (1964 files, best of 4 warm runs): Go +104.5 ms, TypeScript 126.1 ms. +``` + +Incorrect: + +```markdown +The Go validator is significantly faster. + + Used by thousands of teams. + +``` + +--- + +## No Em Dashes + +Rule: New or edited prose uses no em dash (`—`), spaced or not, and no double or +spaced hyphen standing in for one (`--`, `-`). Replace each with, in order of +preference, a period, a comma, a colon, or parentheses; restructure when none +fits. Text you did not touch keeps its punctuation, and so do verbatim quotes of +program output, step names and titles. Convert the sentences you are already +editing; do not reformat a file you were not asked to change. + +Correct: + +```markdown +Every publish step is idempotent: the npm step skips versions already +published, and asset upload clobbers. The chain can therefore be run again for +the same tag. +``` + +Incorrect: + +```markdown +Every publish step is idempotent — the npm step skips versions already +published, asset upload clobbers — so the chain can be run again for the same +tag. +``` + +--- + +## Borrowed Rhythm + +Rule: Avoid the sentence shapes that models reach for by default: + +- "It's not just X, it's Y" and "not only X but also Y" +- A run of clipped fragments for drama ("No config. No setup. No surprises.") +- Every list forced to three items when the content has two or five +- "From X to Y, and everything in between" where X and Y are not a scale +- Aphorisms ("Simplicity is the language of trust") +- Signposting ("Let's dive in", "Here's what you need to know", "In this section + we'll explore") +- Chatbot closers ("I hope this helps", "Let me know if you have questions") +- Upbeat send-offs ("Exciting times ahead", "The future looks bright") +- Recap transitions that open a paragraph by summarizing the last one ("With + this setup complete", "Now that we've explored"); start with the next point +- One dependent idea split into choppy sentences ("Previously this was manual. + Now it's automatic. This saves time."); write it as one sentence +- Spec-sheet voice ("provides", "is configurable", "is explicitly labeled"); say + what the reader does or what happens +- Cold-open paragraphs whose first sentence could stand alone as a heading; + carry the previous subject forward ("Because", "Once") +- Template framing that could open any page ("The question most teams face is + whether"); the angle comes from this page +- Metaphor verbs in place of the literal step ("moves through", "lands", + "hits"); name the step + +Say the thing directly, in as many items as there are. + +Correct: + +```markdown +The dashboard shows the metrics you select. Options come from the selected item. +``` + +Incorrect: + +```markdown +It's not just a dashboard, it's a command center. No guessing. No noise. +``` + +--- + +## Filler and Hedging + +Rule: "In order to" is "to"; "due to the fact that" is "because"; "it is +important to note that" and "it is worth noting" are nothing. One qualifier per +claim: "may" or "usually", never "could potentially possibly". + +Correct: + +```markdown +Spawning it would mean shipping and installing a program to talk to a struct +already in memory. +``` + +Incorrect: + +```markdown +Spawning it would mean shipping and installing a program in order to talk to a +struct already in memory. +``` + +--- + +## Name the Actor + +Rule: When the actor is known, the sentence names it. "The pipeline publishes +the platform packages first", not "the platform packages are published first". +Passive is fine when the actor is unknown, irrelevant or deliberately withheld +("the server was restarted at 03:00"); a quick test is to append "by monkeys" +and see whether the sentence still parses. Do not give abstractions a mind or a +body: a resolver probes, a validator reports, a bundle imports; none of them +"understands", "decides" or "wants", and the browser fetches a URL rather than +being "handed" one. + +--- + +## Neutral Terms + +Rule: Use neutral technical terms in new code, flags and docs: allowlist and +denylist, primary and replica, main. Do not assume the reader's gender, +operating system or access to a paid tool. + +--- + +## Formatting That Shouts + +Rule: Emphasis is written into the sentence, not into the markup. + +- No inline-header bullets that restate the item + (`- **Performance:** it is + faster`). Changelog scope labels + (`**codebase**:`) are a commit convention and stay. +- No bold on every key term. Bold marks a UI element the reader clicks or the + one fact the reader must not miss; reaching for bold to add tone means the + sentence needs rewriting. +- Inline code for paths, file extensions, identifiers, commands and short + snippets (`/api`, `.tsx`, `ctx`): anything that would look wrong outside a + monospace font. +- No quotation marks as emphasis or hedging; quotes are for real quotes, titles + and program output. +- No all-caps clauses inside paragraphs. +- Emojis only where the Emoji Preservation rule already puts them: package + titles and the links footer. Not as bullet decoration in body text. + +--- + +## Audit Before Delivering + +Rule: Before handing over any documentation, read it once as a stranger and +answer two questions: what in this text makes it look generated, and does it +state any fact, number, name, date or quote that is not in the source or in this +codebase. Fix both. Then search the diff for `—`; a hit means the text is not +done. This is the same loop the antislop copywriting skill describes; the +patterns above are its documentation-specific subset, adapted to this codebase's +own conventions (emoji titles and footers, changelog scope labels). diff --git a/.agents/skills/documentation-conventions/references/readmes.md b/.agents/skills/documentation-conventions/references/readmes.md new file mode 100644 index 000000000..436c5beb0 --- /dev/null +++ b/.agents/skills/documentation-conventions/references/readmes.md @@ -0,0 +1,243 @@ +# READMEs and Project Docs + +What this codebase's package READMEs and project documents contain, where they +live, and when they change. Page structure, code examples and doc review are in +docs-structure.md. + +## Contents + +- README Import Pattern +- Emoji Preservation +- Descriptive Text Preservation +- Package README Structure +- Cross-Package References +- Docs Change With the Code +- Where Documentation Lives +- Contributor Setup Stays Documented + +--- + +## README Import Pattern + +Scope: All package README.md files + +Rule: README code examples must use the **namespaced import pattern** +(`import * as pkg from "@scope/pkg"`) and access everything through the +namespace. Never show sub-path direct imports in README examples. Code may +import a published sub-path to keep its module graph small +(javascript-practices: Imports), but READMEs use the primary namespace pattern. +Two cases are exceptions: + +- A CLI-first package maps its `.` export to the CLI's `main.ts` and publishes + its library as `./mod`. Its README imports the library as + `import * as pkg from "@scope/pkg/mod"`, and the module JSDoc of `main.ts` + names that import. The notes linked from this codebase's AGENTS.md list these + packages. +- A module that the package root does not re-export, such as an adapter kept out + of the root so its dependencies load only when used, is shown at the sub-path + the package publishes for it. + +Correct: + +```typescript +import * as functions from "@eserstack/functions"; + +// Access through namespace +const result = await functions.run(async function* () { + /* ... */ +}); +const pipeline = functions.collect(); +const myTask = functions.task.fromPromise(() => fetch("/api")); +const data = await functions.resources.bracket(acquire, use, release); +``` + +```typescript +// A CLI-first package: "." runs the CLI, "./mod" is the library +import * as workflows from "@eserstack/workflows/mod"; + +const registry = workflows.createRegistry(); +``` + +Incorrect: + +```typescript +// ❌ Sub-path imports in README examples +import { collect } from "@eserstack/functions"; +import * as resources from "@eserstack/functions/resources"; +import * as task from "@eserstack/functions/task"; +``` + +**Why:** the reader copies the form the README shows, and the root namespace +reaches the whole public API with one import. A CLI-first package's released +versions already resolve `.` to the CLI, so moving the library there would +change what that import loads for every consumer, a breaking change; its library +takes the `/mod` path instead (backlog TASK-101 records the decision). + +--- + +## Emoji Preservation + +Scope: All documentation files + +Rule: Never remove existing emojis from files. Package README titles use emojis +(e.g. 🧱, ⚡) and footers use 🔗. Preserve them when editing. + +Correct: + +```markdown +# 🧱 @scope/primitives + +... + +## 🔗 Links +``` + +Incorrect: + +```markdown +# @scope/primitives + +... + +## Links +``` + +--- + +## Descriptive Text Preservation + +Scope: All documentation files + +Rule: Never strip descriptive details, comments, or explanatory text when +rewriting documentation. If the original says "Safe acquire-use-release patterns +with guaranteed cleanup", do not shorten it to "Safe acquire-use-release +patterns". Preserve all descriptive context: details like "(LIFO cleanup)" and +"in reverse order" exist for a reason. + +Correct: + +```markdown +Safe acquire-use-release patterns with guaranteed cleanup (LIFO order) +``` + +Incorrect: + +```markdown +Safe acquire-use-release patterns +``` + +--- + +## Package README Structure + +Scope: All package README.md files + +Rule: Each package README should include: + +1. **Title** with emoji and package name +2. **Vision/description**: what the package does and its design philosophy +3. **Quick Start**: the most common use case first, in the fewest lines that run + as pasted, with namespace imports. Options, adapters and advanced composition + come in the API sections, not in the first example +4. **API sections**: grouped by category with examples +5. **Links footer** with 🔗 emoji + +The vision paragraph under the title is the page's opening summary +(docs-structure.md: Openings, Sections and Headings): one paragraph saying what +the package does and for whom. + +--- + +## Cross-Package References + +Scope: Documentation that explains how packages relate + +Rule: State what each layer provides and how the layers differ, and show each +import from the package that owns it, still as a namespace import of the package +root (or of `/mod` for a CLI-first package, README Import Pattern). + +Correct: + +```typescript +// Types and constructors come from @eserstack/primitives +import * as primitives from "@eserstack/primitives"; +// Pure FP utilities come from @eserstack/fp +import * as fp from "@eserstack/fp"; + +const ok = primitives.results.ok(42); +``` + +Incorrect: + +```typescript +import * as results from "@eserstack/primitives/results"; // sub-path import +``` + +--- + +## Docs Change With the Code + +Scope: Any change to a public export, CLI command or flag, config key, +environment variable, daemon endpoint, or install and first-run steps + +Rule: The documentation diff ships in the same change as the code diff; it is +not a follow-up. Before marking the task done, find each doc that describes the +changed surface and make it describe the new state: + +- Exported function or type: its JSDoc in the package, which is its API + reference in editors. Run this codebase's docs-lint script + (`eser codebase validate-docs`), which reports an export without JSDoc. +- Package usage: the package README Quick Start and API sections. +- CLI command or flag: the command's help text and the README that shows it. +- Install, configuration or first run: the root README and CONTRIBUTING.md. +- Environment variable: the package README lists its name, purpose, whether it + is required, its default and an example value. Environment-specific URLs come + from configuration or environment variables, never from literals in code. +- Removed or changed behavior: delete or rewrite the sentences that describe the + old behavior. Descriptive Text Preservation protects details that are still + true, not text that has become false. + +README examples run as pasted: imports included, no elided setup the reader must +guess. + +**Why:** a feature whose docs lag the code does not exist for the reader who +finds it later, and stale instructions cost more than missing ones. + +--- + +## Where Documentation Lives + +Scope: Adding or moving documentation + +Rule: Put a document where its kind already lives, and do not create a new +top-level docs file when one of these fits: + +| Location | Holds | +| -------------------------- | ------------------------------------------------- | +| `README.md` | Project overview, install, first run | +| Each package's `README.md` | Package usage (Package README Structure) | +| `ARCHITECTURE.md` | Architecture overview | +| `SECURITY.md` | Security policy and reporting | +| `CONTRIBUTING.md` | Contributor setup and workflow | +| `docs/adr/` | Architectural decisions (architecture-guidelines) | +| Generated API reference | Built from JSDoc; never edited by hand | +| `CHANGELOG.md` | Generated from commits at release | + +This codebase may keep `ARCHITECTURE.md`, `SECURITY.md` or `CONTRIBUTING.md` +under `.github/`; AGENTS.md says where. When the same document exists twice, a +change updates both until one is made canonical. + +--- + +## Contributor Setup Stays Documented + +Scope: Changes that add a tool, a required environment variable, a service, or a +setup step + +Rule: A first-time contributor goes from `git clone` to a passing full gate (the +one named in AGENTS.md) using only CONTRIBUTING.md. A change that adds a +prerequisite or a step updates CONTRIBUTING.md in the same change, and prefers +folding the step into this codebase's init script over documenting another +manual step. When an ADR sets a first-run target, install and first-run changes +stay within it. A setup failure prints what is missing and the command that +fixes it. diff --git a/.agents/skills/enrichment/SKILL.md b/.agents/skills/enrichment/SKILL.md new file mode 100644 index 000000000..68e34c250 --- /dev/null +++ b/.agents/skills/enrichment/SKILL.md @@ -0,0 +1,42 @@ +--- +name: enrichment +description: "Interviews the user about a plan, decision or idea in rounds, level by level down a decision tree, until both share one understanding; facts are researched by sub-agents, decisions stay with the user. Use when the user asks to enrich, grill or stress-test a plan, and offer it when drafting a PRD, spec, ADR or backlog item whose content has open decisions. Not for a one-off comparison." +--- + +# Enrichment + +A deliberate interview about a plan, decision or idea that continues until no +decision is left open; nothing is built during it. It starts in two ways: the +user asks for it, or the agent offers it while drafting a plan (a PRD, spec, ADR +or backlog item) whose content has open decisions. The short clarification +before a normal task stays in workflow-practices: Ask, Assume or Proceed. + +## Always + +- When drafting a plan with more open decisions than one round of clarification + settles, offer an enrichment session instead of guessing; start it only when + the user accepts +- Model the topic as a design tree: each decision branches into the decisions + that depend on it, level by level from goals down to details +- When a level of the tree is settled, stop and ask whether to go into the next + level of detail or finish at this depth +- Each round asks the whole frontier, every open decision whose prerequisites + are settled, numbered, each with a recommended answer and its reason +- Never ask a question whose answer depends on another question still open in + the same round; it waits for a later round +- Facts are the agent's job: look up anything this codebase, its tools or docs + can answer, with sub-agents in parallel, and never ask the user for it +- A pending lookup blocks only the questions downstream of it; ask the rest of + the frontier now +- Decisions are the user's: put each one to them, and wait for the answers + before the next round +- The session ends when the frontier is empty, or when the user stops or + declines the next level; then summarize the settled tree, list what is still + open, and write the result into the artifact being drafted +- Do not act on the outcome until the user confirms the summary + +## References + +| File | Read when | +| ----------------------------------------------------- | ---------------------------------------------------------------- | +| [interview-rounds.md](references/interview-rounds.md) | Running a session: starting it, levels, rounds, facts, ending it | diff --git a/.agents/skills/enrichment/evals/triggers.json b/.agents/skills/enrichment/evals/triggers.json new file mode 100644 index 000000000..8912716fb --- /dev/null +++ b/.agents/skills/enrichment/evals/triggers.json @@ -0,0 +1,74 @@ +[ + { + "query": "enrich this plan for the notification system", + "should_trigger": true + }, + { + "query": "grill me on the caching design before we build anything", + "should_trigger": true + }, + { + "query": "bu fikri enrich edelim", + "should_trigger": true + }, + { + "query": "stress-test my migration plan to pgx", + "should_trigger": true + }, + { + "query": "interrogate this idea until nothing is left open", + "should_trigger": true + }, + { + "query": "I want every open decision on the plugin system surfaced before we start", + "should_trigger": true + }, + { + "query": "planımı sorgula, tüm kararları tek tek çıkaralım", + "should_trigger": true + }, + { + "query": "poke holes in this release strategy and ask me everything you need", + "should_trigger": true + }, + { + "query": "add caching to the API", + "should_trigger": false + }, + { + "query": "what does the release command do", + "should_trigger": false + }, + { + "query": "fix the failing Go test", + "should_trigger": false + }, + { + "query": "write a README for the cache package", + "should_trigger": false + }, + { + "query": "which skills apply to this task", + "should_trigger": false + }, + { + "query": "review this PR and rank the findings", + "should_trigger": false + }, + { + "query": "make the CLI faster", + "should_trigger": false + }, + { + "query": "summarize the security report", + "should_trigger": false + }, + { + "query": "can we discuss the pros and cons of pnpm vs npm here", + "should_trigger": false + }, + { + "query": "plan the implementation of TASK-23", + "should_trigger": false + } +] diff --git a/.agents/skills/enrichment/references/interview-rounds.md b/.agents/skills/enrichment/references/interview-rounds.md new file mode 100644 index 000000000..9728cba47 --- /dev/null +++ b/.agents/skills/enrichment/references/interview-rounds.md @@ -0,0 +1,178 @@ +# Interview Rounds + +How an enrichment session runs, round by round. + +## Contents + +- Starting a Session +- The Design Tree +- Levels and Checkpoints +- The Frontier +- Round Format +- Facts and Sub-Agents +- Processing Answers +- Ending the Session + +--- + +## Starting a Session + +Scope: When an enrichment session begins + +Rule: A session starts in one of two ways. + +- **The user asks** ("enrich this", "grill me on the plan"): start with the + first round. +- **The agent offers it** while drafting a plan: a PRD, a spec, an ADR, the + content of a backlog item, or any other artifact whose content decides what + gets built. Offer it when the draft has more open decisions than one round of + clarification settles, or when a guess would change the scope. Say how many + decisions are open and which area they are in, and ask whether to run a + session; if the user declines, list the open decisions as explicit assumptions + in the draft (workflow-practices: Stating Assumptions). + +Correct: + +``` +The spec for the notification system has about eight open decisions (delivery +channels, retry policy, storage, user preferences). Shall I run an enrichment +session before writing it, or draft it now with those as stated assumptions? +``` + +--- + +## The Design Tree + +Scope: The start of a session + +Rule: Before the first round, sketch the decisions the topic contains and how +they depend on each other: a storage decision depends on the data model, a +migration plan on the storage decision. Keep the tree in your working notes, not +in the reply; the user sees it only as rounds and, at the end, as the summary. +Leave out what this codebase, its skills or its linters already decide; those +are facts, not decisions. + +**Why:** asking in dependency order is what keeps a round answerable without +guessing, and the tree is what shows when a branch is still unvisited. + +--- + +## Levels and Checkpoints + +Scope: Moving from one depth of the tree to the next + +Rule: Work the tree level by level: first the goals and scope, then the +approach, then the details of each part. When every decision of the current +level is settled, pause before going deeper: give a short summary of the level, +say which areas the next level would cover and roughly how many questions it +holds, and ask whether to continue into the details or stop at this depth. + +``` +Level 1 is settled: scope, channels (email and in-app) and storage (SQLite). +Level 2 would cover retry and backoff, the preference model and the delivery +API, around nine questions. Go into the details, or stop here and write the +spec at this depth? +``` + +A plan that is only needed at the level of scope stops here, and the remaining +levels become open items in the artifact. A user who wants the full picture +continues. + +**Why:** the right depth differs per plan; asking at each level keeps a backlog +item from turning into a design document, and a design from stopping before its +details. + +--- + +## The Frontier + +Scope: Choosing the questions of a round + +Rule: The frontier is every open decision of the current level whose +prerequisites are all settled. Ask all of it in one round, most consequential +first. A decision that depends on an answer you have not heard yet is not on the +frontier, even when it seems likely; asking it now forces the user to answer a +hypothetical. + +--- + +## Round Format + +Scope: Every round + +Rule: Number questions across the whole session (the second round continues at +Q5, not Q1), so an answer like "Q6: B" stays unambiguous. Each question has a +short title, the context the user needs, the options when there are discrete +ones, and your recommended answer with its reason. Use this text format rather +than a question tool, which limits how many questions and how much context fit. + +``` +❓ **Q1** - **Storage for session history**: Sessions are currently kept in +memory and lost on restart. Options: +A. SQLite file next to the service's state +B. PostgreSQL, like the other services +C. Keep in memory, add an export command + +➡️ A: the service runs on developer machines where PostgreSQL is not a given, +and a pure-Go SQLite driver is already a dependency (`go.mod`), so nothing new +is added. + +--- + +❓ **Q2** - **Retention**: ... + +➡️ ... +``` + +Write questions in the conversation's language. Anything written into this +codebase afterwards is English (workflow-practices: Language). + +--- + +## Facts and Sub-Agents + +Scope: Questions that need information from the environment + +Rule: When a frontier question needs a fact (what the code does today, which +dependency is already present, what a tool supports), look it up instead of +asking. For a lookup that takes more than a quick search, dispatch a sub-agent +and keep going: ask the frontier questions that do not depend on it now, and the +dependent ones in the round after the result arrives. Report the facts you found +in the round where they matter, with file:line, so the user decides on the same +information you have. + +**Why:** a user asked for something the code already answers loses time and may +answer wrong; a round held back for one lookup stalls every independent +decision. + +--- + +## Processing Answers + +Scope: Between rounds + +Rule: + +- Mark each answered decision settled and recompute the frontier: settled + decisions unblock the ones that depended on them. +- "Your recommendation" or "ok" settles a question with the recommended answer. +- An unclear or partial answer stays open and comes back, rephrased, in the next + round. +- An answer that contradicts an earlier decision reopens that branch: say which + earlier answer it conflicts with and ask which one holds. +- A new idea from the user adds a branch to the tree. + +--- + +## Ending the Session + +Scope: When the frontier is empty, or the user stops or declines the next level + +Rule: The session is complete when every branch of the chosen depth has been +visited and nothing is silently assumed. Then give a summary of the settled +tree: each decision and its answer, grouped by branch, followed by what is still +open. If the user stops early, the open decisions become explicit assumptions in +that summary. Ask the user to confirm it. After they do, write the result into +the artifact the session was for (the PRD, spec, ADR or backlog item), with the +open items listed in it; any code change after that follows the normal task +workflow (workflow-practices). diff --git a/.agents/skills/eser-rules-manager/SKILL.md b/.agents/skills/eser-rules-manager/SKILL.md new file mode 100644 index 000000000..8d82b6cea --- /dev/null +++ b/.agents/skills/eser-rules-manager/SKILL.md @@ -0,0 +1,52 @@ +--- +name: eser-rules-manager +description: "Picks the skills a task needs and maintains the rules in .agents/skills. Use when starting any task to choose skills, when the user states a preference or a rule to remember, or when creating, changing, splitting, validating or evaluating a skill." +--- + +# Skill Discovery and Rule Management + +Chooses which skills apply to a task, and says how rules are added and kept in +order. + +## Always + +- Load every skill that might apply before the first answer or question, and + name them: a skill loaded late cannot change the plan +- A new rule goes into the one skill that owns the topic; other skills point to + it as `skill-name: Exact Section Title` +- Write only what a strong model would not do by default in this codebase: its + conventions, the tools it uses and the reason for each rule; facts that vary + per codebase go in its AGENTS.md +- Every example follows the other skills' rules +- SKILL.md stays an overview of at most 80 lines that links every reference file + with a "Read when" +- Run `node .agents/skills/eser-rules-manager/scripts/validate-skills.ts --fix` + after every skill change +- A change meant to alter behavior is evaluated against a snapshot of the old + skill + +## Available Skills + +| Skill | Covers | +| --------------------------- | ----------------------------------------------------- | +| `enrichment` | Round-based interview until no decision is left open | +| `handoff` | `/handoff`: a document a fresh session continues from | +| `workflow-practices` | Clarifying, roles, approvals, git, parallel agents | +| `architecture-guidelines` | Hexagonal layers, API surface, testing, ADRs | +| `design-principles` | Pure functions, immutability, interfaces, clocks | +| `coding-practices` | Errors, logging, resources, config, readability | +| `javascript-practices` | TS/JS modules, types, async, tests | +| `frontend-practices` | React, UI, composition, bundling, a11y, laroux | +| `go-practices` | Go conventions, context, data structures, perf | +| `security-practices` | Secrets, input, SSRF, injection, LLM trust boundary | +| `tooling-standards` | Tooling, dependencies, scripts, naming, packages | +| `documentation-conventions` | READMEs, docs structure, prose style, doc review | +| `release-and-ci` | Versions, releases, publishing, workflows, deploys | + +## References + +| File | Read when | +| ----------------------------------------------------- | -------------------------------------------------- | +| [skill-discovery.md](references/skill-discovery.md) | Unsure which skills a task needs | +| [skill-format.md](references/skill-format.md) | Writing or restructuring a skill or a rule | +| [skill-evaluation.md](references/skill-evaluation.md) | Testing that a skill changes behavior and triggers | diff --git a/.agents/skills/eser-rules-manager/evals/triggers.json b/.agents/skills/eser-rules-manager/evals/triggers.json new file mode 100644 index 000000000..70d8b870d --- /dev/null +++ b/.agents/skills/eser-rules-manager/evals/triggers.json @@ -0,0 +1,66 @@ +[ + { + "query": "remember: never use em dashes in docs", + "should_trigger": true + }, + { + "query": "add a rule that Go files must not use init", + "should_trigger": true + }, + { + "query": "create a skill for UI conventions", + "should_trigger": true + }, + { + "query": "the go-practices skill is too long, split it", + "should_trigger": true + }, + { + "query": "artık commit mesajlarında scope zorunlu olsun, kural olarak ekle", + "should_trigger": true + }, + { + "query": "evaluate whether the security skill actually triggers", + "should_trigger": true + }, + { + "query": "which skills should I load for this task", + "should_trigger": true + }, + { + "query": "validate the skills after my edit", + "should_trigger": true + }, + { + "query": "write a README", + "should_trigger": false + }, + { + "query": "fix the lint error", + "should_trigger": false + }, + { + "query": "cut a release", + "should_trigger": false + }, + { + "query": "add a GitHub Action", + "should_trigger": false + }, + { + "query": "explain the hexagonal layers", + "should_trigger": false + }, + { + "query": "write a Go test", + "should_trigger": false + }, + { + "query": "commit these", + "should_trigger": false + }, + { + "query": "what's the SSRF rule", + "should_trigger": false + } +] diff --git a/.agents/skills/eser-rules-manager/references/skill-discovery.md b/.agents/skills/eser-rules-manager/references/skill-discovery.md new file mode 100644 index 000000000..ea4cc0e72 --- /dev/null +++ b/.agents/skills/eser-rules-manager/references/skill-discovery.md @@ -0,0 +1,58 @@ +# Skill Discovery + +Which skills to load for a task, and in what order. + +--- + +## Load Before Responding + +Scope: Every task, including questions and clarifying questions + +Rule: Before the first answer, question or file read, load every skill that +might apply (a small chance is enough) and say which ones you are applying: +`Applying skills: workflow-practices, go-practices, security-practices`. A skill +loaded after the plan is made cannot change the plan. Loading one that turns out +not to matter costs little. + +Look past the obvious one. "Write a Go service for notifications" needs +go-practices, and also architecture-guidelines (layers), security-practices +(input and secrets) and workflow-practices (steps and gates). + +**Why:** agents tend to under-load skills; the ones missed are the ones whose +rules were broken. + +--- + +## Skill Trigger Keywords + +| Skill | Trigger keywords and patterns | +| --------------------------- | ------------------------------------------------------------------------------------ | +| `enrichment` | enrich, grill, stress-test, interrogate a plan or idea | +| `handoff` | user-invoked only: `/handoff [next focus]` | +| `workflow-practices` | any file change, vague request, review, push, tag, delete | +| `architecture-guidelines` | structure, layers, hexagonal, ADR, test strategy | +| `design-principles` | pure function, immutability, interfaces, clock, cancel | +| `coding-practices` | errors, logging, timeouts, config, naming, comments | +| `javascript-practices` | .ts, .js, imports, types, async, TS tests | +| `frontend-practices` | .tsx, React, components, forms, styles, a11y, bundle, laroux | +| `go-practices` | .go, pkg/ajan, context, golangci-lint, benchmarks | +| `security-practices` | auth, secret, token, input, SSRF, injection, LLM input | +| `tooling-standards` | dependency, package.json, new package, file name, license header | +| `documentation-conventions` | README, docs, JSDoc, changelog prose, backlog text | +| `release-and-ci` | release, version bump, npm publish, install script, workflow, CI, Dockerfile, deploy | +| `eser-rules-manager` | new rule, preference, create or change a skill, evals | + +--- + +## Order + +When several skills apply, apply them in this order: + +1. Process: workflow-practices (clarifying, approvals, steps and gates). +2. Design and implementation: architecture-guidelines, design-principles, the + language skill, coding-practices, security-practices. +3. Delivery: release-and-ci. + +Process and safety skills are followed as written (commit policy, quality gates, +release steps). Design and language skills state defaults that the code around +the change may refine. diff --git a/.agents/skills/eser-rules-manager/references/skill-evaluation.md b/.agents/skills/eser-rules-manager/references/skill-evaluation.md new file mode 100644 index 000000000..a16513e77 --- /dev/null +++ b/.agents/skills/eser-rules-manager/references/skill-evaluation.md @@ -0,0 +1,197 @@ +# Skill Evaluation + +How to show that a new or changed skill changes behavior, and that its +description makes it load when it should. + +## Contents + +- Test First +- When to Evaluate +- Behavior Evals +- Trigger Evals +- Improving From Results +- Pressure and Model Coverage +- Watch How Agents Use the Skill +- Before Sharing + +--- + +## Test First + +Scope: Every new skill and every rule meant to change behavior + +Rule: Observe the failure before writing the fix, as with code: + +1. **Red.** Run realistic tasks without the skill (or against a snapshot of the + old version). Record what went wrong and the agent's own words when it talked + itself out of a rule. +2. **Green.** Write the least text that addresses those failures, then rerun. +3. **Refactor.** When new excuses appear, answer them in the rule's reason or an + anti-rationalization note, and rerun until the runs comply. + +**Why:** a rule written for an imagined failure costs context in every session +and may not address what agents actually get wrong. + +--- + +## When to Evaluate + +Scope: Creating a skill, or a change to a skill that alters what an agent does + +Rule: Evaluate when the change is meant to change behavior: a new rule, a +reworded rule that agents were ignoring, a new or rewritten description. Skip it +for typo fixes, link fixes and formatting. Rules whose result is objectively +checkable (a file exists, a command ran, a pattern is absent from the diff) get +assertions. Rules about judgment or prose style get a human read of the outputs +instead of forced assertions. + +**Why:** a skill that reads well but does not change what an agent does is noise +in every context it loads into. + +--- + +## Behavior Evals + +Scope: Checking that the skill's rules are followed + +Rule: Write 2-3 realistic task prompts, the kind a person would type in this +codebase, and run each one twice in the same turn with separate subagents: + +- **With skill:** the subagent reads the skill under test. +- **Baseline:** for a new skill, no skill at all. For a changed skill, the old + version: snapshot it before editing and point the baseline at the snapshot. + +```bash +cp -r .agents/skills/ etc/temp/_skill-evals//snapshot/ +``` + +Launch both runs together so they finish around the same time. While they run, +write the assertions. Each assertion is objectively verifiable and named so a +reader knows what it checks. After the runs, grade each assertion against the +output with a pass or fail and one line of evidence (a file path, a quoted line, +a command result). Check assertions with a script when possible. Then read the +transcripts, not only the final output. + +Keep the work outside the tracked tree, in a gitignored scratch directory such +as +`etc/temp/_skill-evals//iteration-//{with_skill,baseline}/`. +Start the eval directory's name with `_`: Go's `./...` skips such directories, +so Go files an eval writes there are never compiled. Commit only the eval +definitions: `.agents/skills//evals/evals.json`. + +```json +{ + "skill_name": "workflow-practices", + "evals": [ + { + "id": 1, + "name": "bugfix-writes-failing-test-first", + "prompt": "the config merge drops a timeout of 0 from the env, fix it", + "assertions": [ + "A test reproducing the zero-value case is added", + "The report names the root cause with file:line" + ] + } + ] +} +``` + +A skill passes when the with-skill runs pass assertions that the baseline fails. +An assertion that passes in both configurations does not discriminate; replace +it or drop it. + +--- + +## Trigger Evals + +Scope: A new skill, or any change to a `description` + +Rule: The description is what decides whether the skill loads at all. Write +16-20 realistic queries and store them in +`.agents/skills//evals/triggers.json`: + +- 8-10 should trigger: different phrasings of the same need, formal and casual, + some that never name the skill's topic directly. +- 8-10 should not trigger: near misses that share keywords but need a different + skill. "Write a fibonacci function" is not a useful negative for a release + skill; "bump zod to 4.1 in the config package" is, because it shares "bump" + but is a dependency update. + +```json +[ + { "query": "cut v4.6.0 and push the tag", "should_trigger": true }, + { "query": "bump zod to 4.1 in the config package", "should_trigger": false } +] +``` + +Queries must be substantive. An agent does not consult a skill for a one-step +request it can do directly, so "read file X" tests nothing. Check the +description against the set. When skill-creator is installed, its +`scripts/run_loop.py` automates this with a held-out test split; take its best +description by test score, not train score. + +--- + +## Improving From Results + +Scope: The iteration after a failed or mixed eval + +Rule: + +- Generalize. Fix the reason a run went wrong, not the specific prompt; a skill + tuned to three prompts fails on the fourth. +- Remove text that is not pulling its weight. If transcripts show agents + spending time on something the skill told them to do that did not help, cut it + and rerun. +- Explain the reason. When an agent ignored a rule, adding capital letters + rarely helps; saying why the rule exists usually does. +- Bundle repeated work. If every run wrote the same helper script, put it in the + skill's `scripts/` and tell the skill to run it. + +Rerun into a new `iteration-/` with the same baseline, and stop when the +with-skill runs pass, the owner is satisfied, or iterations stop improving. + +--- + +## Pressure and Model Coverage + +Scope: Discipline rules (tests, commits, quality gates) and skills used by +several models + +Rule: Test discipline rules under the pressure that breaks them, not in calm +conditions. Combine at least three pressures in one prompt: time ("we are three +hours in"), sunk cost, the user's insistence ("just make it work"), fatigue at +the end of a long session. A rule that holds only without pressure does not +hold. + +Run the evals on the smallest model that will use the skill as well as the +largest. A skill that works for Opus can be too terse for Haiku; one tuned for +Haiku can over-explain to Opus. + +--- + +## Watch How Agents Use the Skill + +Scope: After a skill has been in use + +Rule: Read transcripts of real tasks, not only eval outputs: + +- Files read in an unexpected order: the References table does not say clearly + when to read what. +- A reference file never opened: it is not needed, or its "Read when" is wrong. +- The same file opened on every task: its core belongs in `## Always`. +- A rule stated in the skill but not followed: give the reason, or move it + higher; do not add capitals. + +--- + +## Before Sharing + +- [ ] `validate-skills.ts` passes without errors +- [ ] The description says what and when, in the third person +- [ ] SKILL.md links every reference file with a "Read when" +- [ ] Every path, command and validator named in the skill exists +- [ ] Every example follows the other skills' rules +- [ ] Each rule says why it exists +- [ ] A baseline run failed an assertion that the with-skill run passes +- [ ] The description was checked against should-trigger and near-miss queries diff --git a/.agents/skills/eser-rules-manager/references/skill-format.md b/.agents/skills/eser-rules-manager/references/skill-format.md new file mode 100644 index 000000000..5b28a70af --- /dev/null +++ b/.agents/skills/eser-rules-manager/references/skill-format.md @@ -0,0 +1,288 @@ +# Skill Format + +How a skill in `.agents/skills/` is laid out and written. Based on Anthropic's +skill authoring guidance +(platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices) and +the Claude Code skills reference. + +## Contents + +- Layout +- Frontmatter +- SKILL.md Body +- Reference Files +- What Belongs in a Skill +- Rule Sections +- Scripts +- Validation +- Creating a Skill +- Changing a Skill + +--- + +## Layout + +``` +skill-name/ +├── SKILL.md # Required: overview and navigation, at most 80 lines +├── references/ # Topic files, each read only when its task needs it +│ ├── errors.md +│ └── logging.md +├── scripts/ # Optional: code the skill runs instead of re-deriving +└── evals/ # Optional: evals.json, triggers.json (skill-evaluation.md) +``` + +`.claude` is a symlink to `.agents`, so edit skills under `.agents/skills/` +only. Paths use forward slashes. + +**Why:** an agent sees only the name and description of every skill up front. It +reads SKILL.md when the skill loads and a reference file only when it opens it. +A skill costs context in proportion to what the task actually needs. + +--- + +## Frontmatter + +Scope: The YAML block at the top of SKILL.md + +Rule: Only `name` and `description` are required. The validator allows the +fields of the Agent Skills spec (`license`, `compatibility`, `metadata`, +`allowed-tools`) and two Claude Code fields that other agents can ignore safely: +`disable-model-invocation: true` for a command-style skill that only the user +starts (`/handoff`), and `argument-hint` for its argument. It rejects fields +that change when a skill loads, such as `paths` or `when_to_use`, because other +agents read the same files and would load it differently. + +A user-invoked skill's description is not shown to the model, so it needs no +"Use when" clause; write it for the person reading the `/` menu. + +- `name`: kebab-case, at most 64 characters, equal to the directory name, and + without the words "anthropic" or "claude". Existing names stay as they are: + AGENTS.md, `.agents/settings.json` and other skills refer to them. +- Quote every value in double quotes. An unquoted value containing `:` or `#` is + invalid YAML, and strict loaders such as `npx skills` skip the whole skill; + the validator reports it. +- `description`: third person, at most 1,024 characters, aiming for under 400. + First what the skill covers, then `Use when ...` with concrete triggers: + tasks, file types, commands, and phrasings that never name the topic. When a + nearby skill is the better fit for a near miss, add a "Not for X (use + other-skill)" clause. The description is the only thing an agent sees before + it decides to load the skill, so every "when" signal belongs there, not in the + body. + +Correct: + +```yaml +description: "Go conventions of this codebase, covering errors, logfx logging, context, testing, data structures and performance. Use when writing or reviewing .go files, adding a Go business domain, or fixing golangci-lint findings. Not for releases (use release-and-ci)." +``` + +Incorrect: + +```yaml +description: Helps with Go stuff # no scope, no triggers +``` + +--- + +## SKILL.md Body + +Scope: Everything below the frontmatter + +Rule: SKILL.md is an overview that sends the reader to the right file. Keep it +to at most 80 lines, in this order: + +1. `# Title`, then one or two sentences on scope and what the skill does not + cover. +2. `## Always`: 5 to 12 bullets that apply to every task in the domain. Only + this codebase's own conventions, each with a short reason when the reason is + not obvious. +3. `## References`: a table that links every reference file directly and says + when to read it. + +```markdown +## References + +| File | Read when | +| ----------------------------------- | -------------------------------------------- | +| [errors.md](references/errors.md) | Writing error handling, messages or wrapping | +| [logging.md](references/logging.md) | Adding logs or changing log levels | +``` + +Loaded skill content stays in context for the rest of the session. A line that +does not change behavior costs tokens on every later turn. + +--- + +## Reference Files + +Scope: Files under `references/` + +Rule: + +- **One level deep.** SKILL.md links every reference file. An agent that follows + a link from one reference file to another tends to preview the second one + partially; links between reference files are fine as long as SKILL.md also + links the target. +- **Split by topic.** One file per task area (errors, logging, testing), named + for its content, kebab-case. A task should read one or two files, not the + whole skill. Aim for at most 400 lines per file; the validator warns past 500. +- **Contents list.** A file over 100 lines starts with `## Contents` after its + one-line scope, so a partial read still shows everything the file covers. + `validate-skills.ts --fix` writes and refreshes it. +- **Skills read as this codebase's conventions.** The skills are installed into + many codebases (`npx skills add eser/stack`), and in each one they read as the + conventions of the codebase they are installed in. A skill says "this + codebase" and states its rules as how this codebase works, never as advice + from outside and never as one specific repository's rules ("for eserstack", + "eserstack's"). The tools of the eser ecosystem (the `eser` CLI, + `.eser/manifest.yml`, ajan, laroux, `@eserstack/*` packages) are named as the + tools this codebase uses. Facts that differ from one codebase to another (its + paths, package names and counts, generator scripts, workflow names, the exact + gate command) stay out of skills: they live in this codebase's `AGENTS.md`, + which every session loads, or in `docs/agents/.md` files linked from + it, and a skill points there ("the gate named in this codebase's AGENTS.md"). +- **One owner per topic.** A rule lives in exactly one skill. Other skills point + to it as `skill-name: Exact Section Title`; the validator checks that the + title exists. + +--- + +## What Belongs in a Skill + +Scope: Every rule and example + +Rule: Assume the reader is a strong model. Write down only what it would not do +by default: + +- decisions and conventions that differ from common defaults; +- the tools to use and how (commands, packages, validators), named as choices; +- the reason behind each rule, so it carries over to cases the examples miss. + +Leave out textbook explanations, lists of benefits, API catalogues, and rules a +linter or formatter already enforces (name the tool in one line instead). Check +every path, command and API against the code before writing it. Every example +must follow the other skills' rules too: an example that breaks another rule +teaches the break. + +Give one default rather than a menu of options, and add the exception that +changes it. Use one term per concept across a skill (for example always +"endpoint", never also "route" or "URL"). Do not write dates or "until version +X" into a rule. State the current rule, and put a replaced one under an "Old +patterns" note only if readers still meet it in the code. + +--- + +## Rule Sections + +Scope: `##` sections in reference files + +Rule: Each rule is one section, separated from the next by `---`: + +````markdown +## Rule Title + +Scope: where it applies + +Rule: the rule, in the imperative. + +Correct: + +```go +// example +``` + +Incorrect: + +```go +// counter-example +``` + +**Why:** the reason, in one or two sentences. +```` + +Keep capitalized MUST and NEVER for true invariants (security, data loss, commit +policy). An agent that knows the reason applies a rule better than one told to +obey it in capitals. Follow documentation-conventions: Plain Prose. + +Match specificity to fragility: + +- **Many valid approaches:** state the goal and the constraints in prose. +- **A preferred pattern:** give the pattern and say what may vary. +- **One safe sequence** (releases, migrations): give the exact commands and say + not to change them. + +A rule that agents break under pressure (skipping tests, committing unasked) may +add an anti-rationalization note: the excuse in quotes, then the answer. + +--- + +## Scripts + +Scope: `scripts/` in a skill + +Rule: When agents keep re-deriving the same steps (a validation, a generator, a +checklist run by hand), put the code in `scripts/` and tell the skill whether to +run it or read it. A script runs without loading into context and gives the same +result every time. + +Scripts use Node built-ins or this codebase's own packages, carry the license +header, handle their own errors with messages that say what to fix, justify +their constants, and pass the full gate named in this codebase's AGENTS.md. + +--- + +## Validation + +Validate after creating or changing a skill: + +```bash +node .agents/skills/eser-rules-manager/scripts/validate-skills.ts # all skills +node .agents/skills/eser-rules-manager/scripts/validate-skills.ts --fix # also rewrite Contents lists +``` + +Pass skill directories to check only those. Errors exit 1; warnings do not. + +| Check | Level | Requirement | +| -------------------- | ------- | ----------------------------------------------------- | +| Frontmatter | error | Present, only allowed keys | +| name | error | kebab-case, at most 64 chars, equals the directory | +| description | error | At most 1,024 chars, no `<` or `>`, has "Use when" | +| description length | warning | Aim for under 400 chars | +| SKILL.md length | error | At most 80 lines | +| SKILL.md links | error | Every relative link resolves | +| SKILL.md sections | warning | Has `## Always` and `## References` | +| Reference links | error | Every `references/*.md` is linked from SKILL.md | +| Contents list | error | Present and current in reference files over 100 lines | +| Reference length | warning | At most 500 lines | +| Cross-skill pointers | warning | `skill-name: Title` matches a heading in that skill | +| Voice | warning | No "for eserstack" or "eserstack's" framing | +| eser-rules-manager | error | Every skill is in both skill tables | + +--- + +## Creating a Skill + +1. Check whether an existing skill should own the topic. Add a new skill only + when no current scope fits. +2. Capture the intent from the conversation first: the steps taken, the owner's + corrections, the tools used. Ask only for the gaps: what the skill should + make an agent do, when it should load, what a good result looks like. +3. Write the evals before the rules (skill-evaluation.md), then write just + enough to pass them. +4. Create `SKILL.md` and the reference files in the format above. +5. Add the skill to the table in eser-rules-manager SKILL.md, to the trigger + table in skill-discovery.md, and to the skill map in AGENTS.md. +6. Validate, then run the behavior and trigger evals. + +--- + +## Changing a Skill + +1. Keep the name and directory. +2. For a change meant to alter behavior, snapshot the skill first + (`cp -r .agents/skills/ etc/temp/_skill-evals//snapshot/`) as the + eval baseline. +3. Change the existing rule rather than adding a near duplicate. When the rule + belongs to another skill, change it there. +4. Update `## Always` only when the change applies to every task in the domain. +5. Validate with `--fix`, then evaluate the change against the snapshot. diff --git a/.agents/skills/eser-rules-manager/scripts/validate-skills.ts b/.agents/skills/eser-rules-manager/scripts/validate-skills.ts new file mode 100644 index 000000000..ae3be2270 --- /dev/null +++ b/.agents/skills/eser-rules-manager/scripts/validate-skills.ts @@ -0,0 +1,396 @@ +// Copyright 2023-present Eser Ozvataf and other contributors. All rights reserved. Apache-2.0 license. + +/** + * Validates every skill under `.agents/skills/` (or the skill directories + * given as arguments) against the format in references/skill-format.md, and + * checks that the eser-rules-manager tables list every skill. + * + * Uses only Node built-ins so it runs without an install, through Node's + * built-in type stripping: + * node .agents/skills/eser-rules-manager/scripts/validate-skills.ts + * + * `--fix` rewrites the `## Contents` list of every reference file longer than + * the table-of-contents threshold before validating. + * + * Exits 1 when any error is found. Warnings do not fail the run. + * + * @module + */ + +// The script runs on a bare `node` with built-ins only, so it imports `node:` +// modules and prints its report straight to the terminal. +/* oxlint-disable no-console */ + +import * as fs from "node:fs"; +import * as path from "node:path"; +import * as process from "node:process"; + +const MAX_SKILL_LINES = 80; +const MAX_DESCRIPTION = 1024; +// Every description sits in the context of every session; past this it +// usually repeats what the body says. +const TARGET_DESCRIPTION = 400; +// Anthropic's guidance: reference files over 100 lines start with a table of +// contents, so a partial read still shows everything the file covers. +const TOC_THRESHOLD = 100; +const TARGET_REFERENCE_LINES = 500; +const ALLOWED_KEYS = new Set([ + "name", + "description", + "license", + "allowed-tools", + "metadata", + "compatibility", + // Claude Code fields that other agents ignore without changing how the skill + // reads: a command-style skill the user starts, and its argument hint. + "disable-model-invocation", + "argument-hint", +]); + +type Finding = { skill: string; level: "error" | "warning"; message: string }; + +const skillsRoot = path.resolve(import.meta.dirname ?? ".", "../.."); + +/** Headings of the given levels outside fenced code blocks, in order. */ +const headingsAt = (text: string, prefix: RegExp): string[] => { + const headings: string[] = []; + let fence: string | null = null; + for (const line of text.split("\n")) { + const marker = line.match(/^\s*(`{3,}|~{3,})/); + if (marker !== null) { + if (fence === null) fence = marker[1]!; + else if (marker[1]!.startsWith(fence)) fence = null; + continue; + } + const match = fence === null ? line.match(prefix) : null; + if (match !== null) headings.push(line.slice(match[0].length).trim()); + } + return headings; +}; + +const sectionHeadings = (text: string): string[] => headingsAt(text, /^## /); + +const lineCount = (text: string): number => + text.replace(/\n$/, "").split("\n").length; + +const buildContents = (headings: readonly string[]): string => + `## Contents\n\n${headings + .filter((h) => h !== "Contents") + .map((h) => `- ${h}`) + .join("\n")}\n`; + +/** + * Inserts or refreshes the `## Contents` block, which sits after the H1 and + * its intro paragraph and ends at the next `---` or `## ` line. + */ +const withContents = (text: string): string => { + const headings = sectionHeadings(text); + const block = buildContents(headings); + const existing = text.match(/^## Contents\n[\s\S]*?(?=\n---\n|\n## )/m); + if (existing !== null) { + return text.replace(existing[0], block.replace(/\n$/, "")); + } + const firstSection = text.search(/^(---|## )/m); + if (firstSection < 0) return text; + return `${text.slice(0, firstSection)}${block}\n---\n\n${text + .slice(firstSection) + .replace(/^---\n\n/, "")}`; +}; + +const contentsMatches = (text: string): boolean => { + const block = text.match(/^## Contents\n([\s\S]*?)(?=\n---\n|\n## )/m); + if (block === null) return false; + const listed = [...block[1]!.matchAll(/^- (.+)$/gm)].map((m) => m[1]!.trim()); + const actual = sectionHeadings(text).filter((h) => h !== "Contents"); + return ( + listed.length === actual.length && listed.every((h, i) => h === actual[i]) + ); +}; + +const referenceFiles = (dir: string): string[] => { + const refs = path.join(dir, "references"); + if (!fs.existsSync(refs)) return []; + return fs + .readdirSync(refs) + .filter((f) => f.endsWith(".md")) + .sort(); +}; + +const parseFrontmatter = (text: string): Record | null => { + const match = text.match(/^---\n([\s\S]*?)\n---\n/); + if (match === null) return null; + const fields: Record = {}; + for (const line of match[1]!.split("\n")) { + const kv = line.match(/^([A-Za-z][\w-]*):\s*(.*)$/); + if (kv === null) continue; + fields[kv[1]!] = kv[2]!.replace(/^(["'])([\s\S]*)\1$/, "$2"); + } + return fields; +}; + +const validateSkill = (dir: string): Finding[] => { + const skill = path.basename(dir); + const findings: Finding[] = []; + const error = (message: string) => + findings.push({ skill, level: "error", message }); + const warn = (message: string) => + findings.push({ skill, level: "warning", message }); + + const skillFile = path.join(dir, "SKILL.md"); + if (!fs.existsSync(skillFile)) { + error("SKILL.md not found"); + return findings; + } + const text = fs.readFileSync(skillFile, "utf8"); + + const fm = parseFrontmatter(text); + if (fm === null) { + error("SKILL.md has no YAML frontmatter (--- name/description ---)"); + return findings; + } + + for (const line of text.split("\n").slice(1)) { + if (line === "---") break; + const kv = line.match(/^([A-Za-z][\w-]*):[ \t]+(.+)$/); + if (kv === null) continue; + const value = kv[2]!; + // An unquoted YAML scalar may not contain ": " or " #"; strict parsers + // (the skills CLI, the Agent Skills loader) reject the whole file. + if (!/^["'|>]/.test(value) && /: |:$| #/.test(value)) { + error( + `frontmatter "${ + kv[1] + }" is an unquoted value containing ": " or " #", which strict YAML parsers reject; wrap it in double quotes`, + ); + } + } + + for (const key of Object.keys(fm)) { + if (!ALLOWED_KEYS.has(key)) { + error( + `unexpected frontmatter key "${key}"; allowed: ${[...ALLOWED_KEYS].join( + ", ", + )}`, + ); + } + } + + const name = fm["name"] ?? ""; + if (name === "") error("frontmatter is missing name"); + else if (!/^[a-z0-9]+(-[a-z0-9]+)*$/.test(name) || name.length > 64) { + error(`name "${name}" must be kebab-case and at most 64 characters`); + } else if (name !== skill) { + error(`name "${name}" does not match the directory name "${skill}"`); + } + + const description = fm["description"] ?? ""; + if (description === "") error("frontmatter is missing description"); + else { + if (description.length > MAX_DESCRIPTION) { + error( + `description is ${description.length} characters; the limit is ${MAX_DESCRIPTION}`, + ); + } else if (description.length > TARGET_DESCRIPTION) { + warn( + `description is ${description.length} characters; aim for under ${TARGET_DESCRIPTION}`, + ); + } + if (/[<>]/.test(description)) { + error("description contains < or >, which some loaders reject"); + } + const userInvoked = /^(true|yes|on|1)$/i.test( + fm["disable-model-invocation"] ?? "", + ); + if (!userInvoked && !/\buse (it )?(when|for|before)\b/i.test(description)) { + error('description has no "Use when/for ..." trigger clause'); + } + } + + const lines = lineCount(text); + if (lines > MAX_SKILL_LINES) { + error( + `SKILL.md is ${lines} lines; the limit is ${MAX_SKILL_LINES}. Move detail to references/`, + ); + } + + for (const heading of ["## Always", "## References"]) { + if (!text.includes(`\n${heading}`)) warn(`SKILL.md has no "${heading}"`); + } + + for (const [, target] of text.matchAll(/\]\(([^)#\s]+)\)/g)) { + if (/^[a-z]+:/.test(target!)) continue; + if (!fs.existsSync(path.join(dir, target!))) { + error(`SKILL.md links to ${target}, which does not exist`); + } + } + + // Skills are installed into many codebases and read as the conventions of + // whichever one they are in, so "this codebase" is the intended voice. What + // must not appear is framing the skill as one specific repository's rules + // (skill-format.md: Reference Files). + const projectBound = + /\b(for eserstack|in eserstack|eserstack's|eserstack (repository|repo|monorepo))\b/i; + for (const file of [ + "SKILL.md", + ...referenceFiles(dir).map((f) => `references/${f}`), + ]) { + const body = fs.readFileSync(path.join(dir, file), "utf8"); + body.split("\n").forEach((line, index) => { + // Quoted text names the phrase rather than using it. + if (projectBound.test(line.replace(/"[^"]*"/g, ""))) { + warn( + `${file}:${index + 1} frames the skill as eserstack's own rules ("${line + .trim() + .slice( + 0, + 60, + )}"); state the convention for this codebase and point to AGENTS.md for facts`, + ); + } + }); + } + + for (const file of referenceFiles(dir)) { + const ref = `references/${file}`; + if (!text.includes(`](${ref})`)) { + error( + `${ref} is not linked from SKILL.md; link every reference file directly`, + ); + } + const body = fs.readFileSync(path.join(dir, ref), "utf8"); + const refLines = lineCount(body); + if (refLines > TOC_THRESHOLD && !contentsMatches(body)) { + error( + `${ref} is ${refLines} lines and its "## Contents" list is missing or out of date; run with --fix`, + ); + } + if (refLines > TARGET_REFERENCE_LINES) { + warn( + `${ref} is ${refLines} lines; consider splitting it by topic (aim for under ${TARGET_REFERENCE_LINES})`, + ); + } + } + + return findings; +}; + +/** + * Cross-skill pointers are written as `skill-name: Section Title`. Each one + * must name a skill that exists and start with one of its section headings. + */ +const validateCrossReferences = (dirs: readonly string[]): Finding[] => { + const findings: Finding[] = []; + const headingsBySkill = new Map(); + for (const dir of dirs) { + const files = [ + "SKILL.md", + ...referenceFiles(dir).map((f) => `references/${f}`), + ]; + headingsBySkill.set( + path.basename(dir), + files.flatMap((f) => + headingsAt(fs.readFileSync(path.join(dir, f), "utf8"), /^#{2,4} /), + ), + ); + } + const names = [...headingsBySkill.keys()].join("|"); + const pointer = new RegExp(`\\b(${names}): ([A-Za-z\`][^\\n]*)`, "g"); + for (const dir of dirs) { + const skill = path.basename(dir); + const files = [ + "SKILL.md", + ...referenceFiles(dir).map((f) => `references/${f}`), + ]; + for (const file of files) { + const text = fs + .readFileSync(path.join(dir, file), "utf8") + .replace(/[ \t]*\n(?!\n)[ \t]*/g, " "); + for (const [, target, rest] of text.matchAll(pointer)) { + const headings = headingsBySkill.get(target!) ?? []; + if (!headings.some((h) => rest!.startsWith(h))) { + findings.push({ + skill, + level: "warning", + message: `${file} points to "${target}: ${rest!.slice( + 0, + 50, + )}", which matches no section heading in ${target}`, + }); + } + } + } + } + return findings; +}; + +const fixContents = (dirs: readonly string[]): void => { + for (const dir of dirs) { + for (const file of referenceFiles(dir)) { + const filePath = path.join(dir, "references", file); + const text = fs.readFileSync(filePath, "utf8"); + if (lineCount(text) <= TOC_THRESHOLD || contentsMatches(text)) continue; + fs.writeFileSync(filePath, withContents(text)); + console.log(`updated contents: ${path.basename(dir)}/references/${file}`); + } + } +}; + +/** Every skill must appear in both eser-rules-manager tables. */ +const validateInventory = (skills: readonly string[]): Finding[] => { + const findings: Finding[] = []; + const manager = path.join(skillsRoot, "eser-rules-manager"); + const tables: Array<[string, string]> = [ + ["SKILL.md", "Available Skills table"], + ["references/skill-discovery.md", "Skill Trigger Keywords table"], + ]; + for (const [file, label] of tables) { + const tablePath = path.join(manager, file); + if (!fs.existsSync(tablePath)) continue; + const text = fs.readFileSync(tablePath, "utf8"); + for (const skill of skills) { + if (skill === "eser-rules-manager") continue; + if (!text.includes(`| \`${skill}\``)) { + findings.push({ + skill, + level: "error", + message: `missing from the ${label} in eser-rules-manager/${file}`, + }); + } + } + } + return findings; +}; + +const main = (): number => { + const fix = process.argv.includes("--fix"); + const args = process.argv.slice(2).filter((a) => a !== "--fix"); + const dirs = + args.length > 0 + ? args.map((a) => path.resolve(a)) + : fs + .readdirSync(skillsRoot) + .map((d) => path.join(skillsRoot, d)) + .filter((d) => fs.statSync(d).isDirectory()); + + if (fix) fixContents(dirs); + + const findings = dirs.flatMap(validateSkill); + if (args.length === 0) { + findings.push(...validateInventory(dirs.map((d) => path.basename(d)))); + findings.push(...validateCrossReferences(dirs)); + } + + for (const f of findings) { + console.log(`${f.level === "error" ? "✗" : "!"} ${f.skill}: ${f.message}`); + } + const errors = findings.filter((f) => f.level === "error").length; + const warnings = findings.length - errors; + console.log( + `${dirs.length} skill(s) checked in ${ + args.length > 0 ? path.dirname(dirs[0]!) : skillsRoot + }: ${errors} error(s), ${warnings} warning(s)`, + ); + return errors > 0 ? 1 : 0; +}; + +process.exit(main()); diff --git a/.agents/skills/frontend-practices/SKILL.md b/.agents/skills/frontend-practices/SKILL.md new file mode 100644 index 000000000..5f1c40c7a --- /dev/null +++ b/.agents/skills/frontend-practices/SKILL.md @@ -0,0 +1,49 @@ +--- +name: frontend-practices +description: "React and web UI conventions of this codebase, laroux included: components and composition, state and effects, server components and actions, bundling, accessibility and interface rules. Use when building or reviewing .tsx components (with javascript-practices), pages, forms or styles, accessibility, bundle size or rendering speed. Not for .ts files without UI (use javascript-practices)." +--- + +# Frontend Practices + +How this codebase builds React 19 user interfaces: on laroux when it uses +laroux, and the same way in any other React or browser UI. TypeScript syntax, +imports and tests are in javascript-practices; HTTP hardening is in +security-practices. + +## Always + +- Components take one `props` object, are declared at module level and are named + exports; a default export only where laroux loads `default` (`proxy.ts`, + `laroux.config.ts`) +- Compose explicit variants and compound components instead of adding boolean + mode props; accept `children` rather than `renderX` props +- Derive values during render; effects only synchronize with external systems, + and user actions run in their event handlers +- No `useMemo`, `useCallback` or `memo` without a profile; laroux-bundler does + not run the React Compiler, so structure is what keeps renders cheap +- Every view that loads, lists or submits data has loading, empty, error, + success and partial states +- Keyboard access, visible focus, accessible names and WCAG 2.1 AA contrast on + every surface; semantic elements before ARIA +- Colors, spacing and type come from existing design tokens +- Start independent async work before awaiting it; put slow parts behind + `Suspense` +- Server actions and `route.ts` handlers are public endpoints: validate and + authorize inside each one +- Only the fields a client component renders cross the server/client boundary, + as plain JSON data +- Render conditionally with explicit conditions (`count > 0 ? … : null`), never + `&&` on a number or string +- The micro-optimizations in hot-paths.md apply only to a profiled hot path + +## References + +| File | Read when | +| ------------------------------------------------------------------------- | -------------------------------------------------------------- | +| [components-and-composition.md](references/components-and-composition.md) | Writing a component, designing its props or a component family | +| [rendering-and-state.md](references/rendering-and-state.md) | State, effects, memoization, hydration, browser storage | +| [data-fetching-and-server.md](references/data-fetching-and-server.md) | Fetching data, server components, server actions, caching | +| [bundling-and-loading.md](references/bundling-and-loading.md) | Bundle size, lazy loading, preloading, scripts and assets | +| [interface-guidelines.md](references/interface-guidelines.md) | Building or reviewing UI: states, accessibility, forms, copy | +| [laroux.md](references/laroux.md) | Routes, "use client", server actions, CSS modules on laroux | +| [hot-paths.md](references/hot-paths.md) | Optimizing loops, DOM work or listeners a profile flagged | diff --git a/.agents/skills/frontend-practices/evals/triggers.json b/.agents/skills/frontend-practices/evals/triggers.json new file mode 100644 index 000000000..c71e2ffd0 --- /dev/null +++ b/.agents/skills/frontend-practices/evals/triggers.json @@ -0,0 +1,66 @@ +[ + { + "query": "build a settings page with a form and inline validation errors", + "should_trigger": true + }, + { + "query": "this .tsx modal doesn't return focus when it closes, fix it", + "should_trigger": true + }, + { + "query": "review the dashboard UI for accessibility problems", + "should_trigger": true + }, + { + "query": "the client bundle grew 400 KB after we added the icon library", + "should_trigger": true + }, + { + "query": "typing in the search box lags when the result list is long", + "should_trigger": true + }, + { + "query": "should this component be a server component or \"use client\" in laroux", + "should_trigger": true + }, + { + "query": "style the product card with a CSS module instead of inline styles", + "should_trigger": true + }, + { + "query": "bu React bileşeninde boolean prop'lar çoğaldı, nasıl toparlarız", + "should_trigger": true + }, + { + "query": "add a generic type parameter to parseConfig in the primitives package", + "should_trigger": false + }, + { + "query": "replace the named imports in this .ts file with namespace imports", + "should_trigger": false + }, + { + "query": "convert the `if (!value)` checks in slug.ts to explicit checks", + "should_trigger": false + }, + { + "query": "add a mod.ts entry point that re-exports the public functions", + "should_trigger": false + }, + { + "query": "document the laroux-bundler options in its README", + "should_trigger": false + }, + { + "query": "rewrite this guide page so it reads less like marketing copy", + "should_trigger": false + }, + { + "query": "write a Go HTTP handler that serves the dashboard API", + "should_trigger": false + }, + { + "query": "golangci-lint flags a struct field alignment in this Go file", + "should_trigger": false + } +] diff --git a/.agents/skills/frontend-practices/references/bundling-and-loading.md b/.agents/skills/frontend-practices/references/bundling-and-loading.md new file mode 100644 index 000000000..be1b3dd52 --- /dev/null +++ b/.agents/skills/frontend-practices/references/bundling-and-loading.md @@ -0,0 +1,188 @@ +# Bundling and Loading + +What ships to the browser and when it loads: imports and tree shaking, import +paths, lazy loading, preloading, resource hints, scripts and static assets. + +## Contents + +- Namespace Imports and Tree Shaking +- Statically Analyzable Paths +- Load Heavy and Optional Code on Demand +- Preload on User Intent +- Resource Hints +- Scripts Never Block Parsing +- Small SVG Assets + +--- + +## Namespace Imports and Tree Shaking + +Scope: Imports in code that ships to the browser + +Rule: Keep namespace imports (javascript-practices: Imports) and access their +members by name (`icons.Check`). Rolldown, the backend of laroux-bundler, drops +the unused exports of a namespace whose members are accessed by name; an access +by computed key (`icons[name]`) keeps the whole module. When the choice is +dynamic, write an explicit map of the members you use. For a large third-party +library (an icon set, a component kit) whose entry pulls in modules with side +effects, or that a dev server or test runner loads unbundled, import from the +library's documented per-module entry point instead, and compare the bundle size +before and after. + +Correct: + +```tsx +import * as icons from "lucide-react"; + +const STATUS_ICONS = { done: icons.Check, failed: icons.X } as const; + +