diff --git a/AGENTS.md b/AGENTS.md index 16abed758..7c1b4c07d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,53 +1,39 @@ # Repository guidance +## Scope and completion + +Carry requested changes through the relevant implementation, inspection, and +verification. Resolve failures introduced by the change before reporting done. +Continue independent authorized work while a missing decision blocks another +part; a skill recipe alone is not a reason to expand scope or stop for approval. + +Load guidance for the task it actually covers. Keep skill descriptions concise +and specific, and put substantial conditional procedures in linked references. +Preserve operational invariants; avoid blanket document reads, arbitrary output +counts, and repeated checks without new evidence. + ## Code hygiene -- **Extract and rewire in one change.** Every dead module this repo has had to - delete was born the same way: a refactor lifted helpers into a new file and - never switched the original over, so the monolith kept its inline copy and - the new file had zero importers from its first commit. If a commit adds - `foo-codec.ts`, the same commit deletes the code it replaced and leaves - `foo.ts` importing it. The follow-up PR that "wires it up" does not arrive. -- **A module with no production importer is not delivered.** This repo's - dominant failure mode is a thoroughly tested service that nothing mounts. - Before believing a capability exists, find the production caller, not the - test. Before opening a PR, confirm every file it adds is reachable from - `src/index.ts`, a route, a CLI entry, or a hook — a passing suite proves - nothing about whether the code runs. -- **Look for the helper before writing it.** `dev/http.ts` owns request and - response helpers (`diagnostic`, `requestError`, `isRequestDiagnostic`, - `responseDiagnostic`, `responseJson`, `singleHeader`, `isJsonRequest`, - `readBody`, `readJsonBody`, `rawPathname`, `decodedOpaqueSegment`); - `core/strict-json.ts`, `core/errors.ts`, `core/paths.ts`, and - `core/freeze.ts` own their equivalents. A route module that defines its own - `readBody` has forked a security-relevant bound that will be fixed in one - copy and not the other. -- **Never copy a helper to dodge an import cycle.** Move it to a leaf module - both sides import — `config/conventional-entry.ts` is the pattern. A comment - explaining why the copy exists documents the debt; it does not discharge it. -- **One class per name.** Two identical `class FooError` declarations in two - modules are not interchangeable: `instanceof` against the wrong one silently - returns `false`, so the `catch` that was supposed to handle it falls through. - Error classes live with the code that throws them, exported once. -- **Delete on sight.** Unreferenced code is not free — it is read during - review, matched by search, and copied by the next author who finds it before - the live version. Removing it is a `patch` changeset, not a project. -- Neither `pnpm lint` nor `pnpm typecheck` reports an unreferenced module, so - check by hand when a change adds or moves files: - - ```sh - # any tracked file that mentions the module, other than itself - git grep -l '' -- ':!repos' - ``` - - One hit means the module only mentions itself and nothing imports it. Watch - for false positives from prose in `docs/**` and from strings that merely - contain the name: `Symbol('epoch-staging')` in `dev/epoch-store.ts` was the - only match for a 343-line dead file, which is why it read as reachable. -- Gate before pushing: `pnpm build && pnpm typecheck && pnpm lint && pnpm - test:unit`. Every Rstest pool and `pnpm typecheck` refuse to start over a - `dist` older than its sources (`scripts/dist-freshness.mjs`), so the build - comes first whatever the change touched. +- Extract and rewire together: migrate production callers and delete the old + implementation in the same change. Added production modules must be reachable + from an application, CLI, route, or hook entrypoint; isolated tests do not + establish that a capability is mounted. +- Reuse the request/response helpers in `dev/http.ts` (`readBody`, `readJsonBody`, + `responseJson`, `singleHeader`, diagnostics and path helpers) and the canonical + `core/strict-json.ts`, `core/errors.ts`, `core/paths.ts`, and `core/freeze.ts`. + Duplicating them can fork security bounds. Resolve import cycles with a shared + leaf module, as in `config/conventional-entry.ts`, rather than copying code. +- Error classes have one exported definition; duplicate classes break + `instanceof` identity across callers. +- When adding, moving, or removing code, check production reachability, including + dynamic loading and public entrypoints. Text-match counts and passing lint or + typechecks alone do not prove a file is unused. Remove confirmed obsolete code + within the requested scope; do not delete unrelated code on sight. +- Match checks to changed behavior. Code changes use the local gate below and + affected integration/packed coverage. For instruction-only edits, validate + syntax, links, and relevant skill rendering or packaging instead of rebuilding + unrelated applications. Rstest and typecheck require fresh `dist`; when running + them, build first and preserve `scripts/dist-freshness.mjs` enforcement. ## Workbench platform scope @@ -66,62 +52,33 @@ ## Generated plugin output -- Generated plugin output is self-contained. The compiler profile in - `packages/agent-bundle/src/build/rslib.ts` (`composeEntryLibConfig`) bundles - every dependency of a generated executable — `output.autoExternal: false`, - `bundle: true`, `splitChunks: false`, no `externals`. Rslib's `node` target - leaves only Node built-ins (and `pnpapi`) external. The compiler service - (`src/build/compiler.ts`, `external-policy.ts`, - `dependency-audit-plugin.ts`) records every `ExternalModule` of every - host-pack, `dist`, and MCP App view compilation and fails the build - (`AB6005`) on anything but a Node built-in, `pnpapi`, or an emitted sibling - — whatever spelling Rspack emitted. The package build's `dist` bundles are - judged by the same rule and then walked like host-pack modules - (`src/build/package-build.ts` reuses `validateJavaScriptModules`), so a - generated executable in a host pack or in `dist` imports nothing but Node - built-ins from outside its tree. MCP App views - (`src/build/mcp-apps.ts`) inline every script and style into one HTML file. - The framework never adds `externals` to a plugin build; the `externals` - handling in `rslib.ts` (`reservedExternalsViolation`, - `guardReservedExternals`) only rejects reserved specifiers in the resolved - externals, which come from the author's `tools` hatch and Rslib's built-in - list, never from the profile. -- No refactor, toolchain upgrade, or "leaner install" change may enable - `autoExternal` or externalize a dependency on the author's behalf. A package - a consumer must install is the author's explicit decision, and an import - kept external through the `tools` hatch is not a way to make it anywhere: - `AB6005` fails such an import in a host pack and in `dist` alike. What - legitimately puts a package under `dependencies` is one of three `AB7014` - evidence sources: `runtimeDependencies` on a prebuilt payload - (`definePrebuilt`), a packed declaration reference, or a consumer-side - install script that names or runs it. The framework's own runtime modules - load no package at run time (`generated-module-evidence.test.ts`), so there - is no framework process-dependency record to read. `AB7015` additionally - requires a specifier a consumer's npm can install. -- Proof is compiler evidence, the persisted record, and packed-process - tests: each compilation's own external and module records are judged before - emission, `build` writes them beside the emitted files as - `agent-bundle.compile-evidence.json` (`src/build/compile-evidence.ts`) and - `validate --artifact` re-checks the record against the file table - (`AB6039`), the prepack gate judges declared dependencies from evidence, - and the packed pool (`pnpm test:packed`) installs the packed tarball into a - clean consumer, builds, removes the project source, and spawns the - generated entry as a real process (`packed-deleted-source`). The - emitted-module walk (`src/build/validate-artifact-modules.ts`) remains - only for what the compiler cannot see: an expression `import()` in a - compiled module (Rslib's profile leaves `import()`, - `require()`, `require.resolve(…)`, `createRequire(…)(…)`, and - `import.meta.resolve(…)` verbatim — no module, no external, no warning), - JavaScript the framework did not compile (`install.mjs`, copied scripts), - and every module of a build with a `tools` hatch (`coverage.rewritable`). - A compiled module the record proves is lexed, not parsed, and each literal - import it still carries is held to the record: a Node built-in or a - recorded external passes, anything else is an import the build was told - to ignore (`rspackIgnore`/`webpackIgnore` — Rspack leaves it verbatim with - no module, external, or warning) and fails `AB6005`. A matching digest - proves the bytes are the compiler's, never that every import in them was - resolved. The walk is not a second self-containment check and must not - grow one: it resolves nothing the record already judged. +Generated executables are self-contained. Preserve these compiler boundaries: + +- `packages/agent-bundle/src/build/rslib.ts` uses `autoExternal: false`, + `bundle: true`, and `splitChunks: false`. Do not externalize dependencies on + the author's behalf. MCP App views inline scripts and styles into one HTML. +- Each compilation judges its own external/module records. Only Node built-ins, + `pnpapi`, and emitted siblings may remain external (`AB6005`). Imports supplied + through the author's `tools` hatch are still subject to `AB6005` and may not + escape bundling. +- Build persists `agent-bundle.compile-evidence.json`; `validate --artifact` + checks it against the file table (`AB6039`). A matching digest proves byte + identity, not that all imports resolved. Preserve the packed test that removes + source and runs the generated executable in a clean consumer. +- Consumer dependencies require `AB7014` evidence: prebuilt + `runtimeDependencies`, packed declaration references, or consumer install + scripts. `AB7015` requires an npm-installable specifier. The framework's runtime + modules do not create implicit process dependencies. +- The emitted-module walk handles what the compiler cannot see: expression + imports, uncompiled/copied scripts, and every module of a build using the + author's `tools` hatch. It must not grow a parallel resolver for records the + compiler already judged. Literal imports left in compiled output still need + recorded authority; ignore comments do not make them safe. + +For changes to bundling or artifact validation, inspect `src/build/compiler.ts`, +`external-policy.ts`, `dependency-audit-plugin.ts`, `compile-evidence.ts`, +`package-build.ts`, and `validate-artifact-modules.ts` under +`packages/agent-bundle`, choosing the files that own the affected behavior. ## Documentation site @@ -173,56 +130,44 @@ ## Pull requests -- Every PR gets a deslop pass before review: read the full diff against - `origin/main` and remove what a human author would not have written — - comments that restate the code or break the file's style, defensive checks - and `try`/`catch` on trusted paths, `as any` / `as unknown as` casts that - only silence the type checker, nesting that early returns would flatten, - helpers that `dev/http.ts` or `core/*` already own, placeholder prose in - the PR body. Behavior stays unchanged unless the pass finds a clear bug. - Record it in the PR body ("Deslop: model, N edits"). -- Every PR gets a self-review before merge: spawn a local reviewer subagent - (`change-risk-reviewer` if available, else `generalPurpose`; a different - model from the author's, e.g. `gpt-5.6-sol-high` or a Claude model — never - a Grok model for review) with the repo - path and the PR number or branch, asking for concrete merge risks only — - bugs, breaking changes, missed tests, doc or changeset gaps — against the - current diff vs `origin/main`. Fix or explicitly dismiss every finding in - the PR description under a "Self-review" section (reviewer model, findings, - disposition), run the reviewer once more after fixes, then merge on green - CI. -- CI green first. Address every review thread, whoever opened it — fix it in - the same PR or reply with a precise reason — and re-check for new threads - after each push until none remain. Only then merge. -- PRs are squash-merged. Review threads left on an already-merged PR must - still be answered, in a follow-up PR. -- `main` requires no status checks (owner decision, 2026-09-06): the local - gate is the merge gate. Before `gh pr merge --squash`, run on a branch that - contains current `origin/main`: `pnpm build && pnpm typecheck && pnpm lint - && pnpm test:unit`, every integration/packed test file the diff touches, - and `pnpm docs:site:build` when `website/` or public exports changed; - paste the commands and their results in the PR body. "Files the diff - touches" means every test that imports or exercises a changed module, not - the files the author expects to matter — when the change moves an output - root, a watcher path, or a public type, run the whole integration pool. - Do not wait for CI on the PR, but do not ignore it either: a PR run that - has already finished red is a failed gate (#656 merged over six red - integration tests its local gate never ran). CI still runs on `main` - after the merge: whoever merged watches - that run and fixes or reverts a red `main` before starting anything else. - A later push to `main` cancels the superseded `CI` and `Package preview` - run (`concurrency` in `ci.yml`, `package-preview.yml`), so watch the tip - commit's run; `docs.yml` and `release.yml` never cancel a `main` run, - since a deploy or publish must not die mid-flight. - `gh pr update-branch` only when GitHub reports the branch as conflicting; - never `--admin`, never force-push `main`. +- Review the actual diff against the integration base for correctness, duplicate + helpers, noisy comments, placeholder prose, and casts or defensive branches + that conceal errors. Preserve real trust-boundary checks. Report material + findings and their resolution; no fixed edit quota or review transcript format + is required. +- Before merging code changes, get an independent reviewer for concrete merge + risks, including missing integration, documentation, or changesets. Prefer + `change-risk-reviewer` when available. Use a different model from the author, + and do not use Grok for review. Review relevant follow-up changes after fixes; + reuse unchanged evidence. Coordinate reviewers with explicit file scope and + preserve unrelated work. +- Address every review thread with a fix or precise reason. Re-check threads + after a push; unresolved threads on an already-merged PR need a follow-up. + PRs use squash merges. Merge only within the user's authorization. +- `main` has no required status checks (owner decision, 2026-09-06), so the local + gate is the merge gate. Before merging code changes, run the following on a + branch that contains current `origin/main`: `pnpm build`, `pnpm typecheck`, + `pnpm lint`, and `pnpm test:unit`, plus integration/packed tests that exercise + changed modules. Output-root, watcher, or public-type changes require the whole + integration pool. Run + `pnpm docs:site:build` when website or public exports changed. Record commands + and results in the PR. Scope instruction-only checks as described above. +- A completed red CI run is a failed gate even when GitHub allows merging. + After an authorized merge, watch the tip commit's CI and prioritize restoring + green `main` before starting new work. Fix or revert failures attributable to + the merge and coordinate unrelated failures with their owners. Superseded + CI/Package preview runs may be cancelled by a newer push; docs/release runs + must finish their deployment or publication. +- Use `gh pr update-branch` only for a reported conflict. Never use `--admin` or + force-push `main`. ## Vendored repos - `repos/` is **read-only reference material**. Do not edit, format, or import from `repos/**`. - Application code imports the published npm package (`effect`), never a path under `repos/`. -- Before writing Effect code, read `repos/effect/LLMS.md` and the linked - `agent-patterns/effect-{stream,scope,concurrency,errors}.md`. +- For unfamiliar Effect behavior, consult `repos/effect/LLMS.md` and the + relevant linked stream, scope, concurrency, or error guidance. Known local + patterns and installed types can answer routine edits without reading them all. - Editor search, file watching, and auto-import exclude `repos/**` (`.vscode/settings.json`). Subtree updates ride the same named chore as the Effect RC re-pin — see `docs/effect-conventions.md`. diff --git a/CLAUDE.md b/CLAUDE.md index 0b83f64e7..8356b3f72 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,6 +1,2 @@ -Repository guidance for this project lives in [AGENTS.md](./AGENTS.md) — read it -first. It is the single source for code hygiene, Workbench scope, public -examples, the docsite, changesets, pull requests, and vendored `repos/`. - -Keep it that way: add project rules to `AGENTS.md`, never here. A second copy -of the guidance is the same duplication the hygiene section exists to prevent. +Read [AGENTS.md](./AGENTS.md) for repository guidance. Keep project rules there +to avoid conflicting copies.