Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .agents/skills/agent-core-dev/orient.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ There is no domain-layer numbering — a domain may import any other domain, gui

## Comment convention

`packages/agent-core-v2/AGENTS.md` bans comments: no file headers, no section banners, no statement-level narration — the code is the source of truth. The only exception is JSDoc attached to exported symbols, which flows into the generated `.d.ts` and the consumers' IDE hover. Tooling directives (`eslint-disable`, `@ts-expect-error`, …) are banned too: fix the underlying lint/type problem instead, and put negative type-safety cases in compiler-asserted fixtures. Scope is carried by the filename: `workspace*.ts` = Workspace, `session*.ts` = Session, `agent*.ts` = Agent, no prefix = App (see service-authoring.md).
`packages/agent-core-v2/AGENTS.md` bans comments entirely: no file headers, no section banners, no statement-level narration, no JSDoc (not even on exported symbols) — the code is the source of truth. The only exception is a load-bearing lint-suppression directive (`oxlint-disable` / `eslint-disable`) for a deliberate pattern; other tooling directives (`@ts-expect-error`, …) are banned: fix the underlying lint/type problem instead, and put negative type-safety cases in compiler-asserted fixtures. Scope is carried by the filename: `workspace*.ts` = Workspace, `session*.ts` = Session, `agent*.ts` = Agent, no prefix = App (see service-authoring.md).

## Red lines (this stage)

- Import via the `#/...` alias (mapped to `src/`); never reach into another domain's internals by relative path.
- Short-lived may inject long-lived; never the reverse.
- No comments — not file headers, not beside statements; exported-symbol JSDoc is the only exception.
- No comments — not file headers, not beside statements, no JSDoc anywhere; `oxlint-disable` / `eslint-disable` are the only exception.
5 changes: 2 additions & 3 deletions .agents/skills/agent-core-dev/service-authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,8 @@ Importing the package therefore fires every `register*` side effect, exactly as

## Comments

- **No comments** (orient.md): no file headers, no statement-level narration; the only exception is JSDoc attached to exported symbols.
- **Methods and fields carry no comments by default.** Well-named identifiers and types say *what*; the code is the source of truth for *how*.
- Write an inline comment only when the *why* is non-obvious (a hidden constraint, a subtle invariant, a workaround). One short line.
- **No comments** (orient.md): no file headers, no statement-level narration, no JSDoc — not on exported symbols either; the only exception is a load-bearing `oxlint-disable` / `eslint-disable` directive.
- **Methods and fields carry no comments.** Well-named identifiers and types say *what*; the code is the source of truth for *how*.
- For unimplemented stubs, throw `NotImplementedError('feature')` rather than `throw new Error('TODO: …')` (errors.md).

## Complete minimal example
Expand Down
2 changes: 1 addition & 1 deletion .agents/skills/agent-core-dev/verify.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Walk the stages you touched and confirm:
- **Design** — scope follows state identity; no `Map<sessionId, …>` at `App`; dependency arrows do not make a foundational layer know an upstream one; no cycle was routed around.
- **Implement** — no `new` on `@IService`-carrying classes; `@IX` on constructor params only (service params after static params); interface + impl carry `_serviceBrand`; decorator names unique; coded errors only; flags for unreleased behavior.
- **Test** — SUT resolved by interface; stubs under `test/`; scope tests re-register after `_clearScopedRegistryForTests()`; teardown through one `DisposableStore`.
- **Files** — no comments (exported-symbol JSDoc excepted); registration runs from the impl file's top level; the new domain is exported from `src/index.ts`.
- **Files** — no comments at all (no JSDoc either; only load-bearing `oxlint-disable` / `eslint-disable` survive); registration runs from the impl file's top level; the new domain is exported from `src/index.ts`.

Then re-read the [global red lines](SKILL.md#global-red-lines) once — they catch most cross-stage mistakes in a single scan.

Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ This is a TypeScript monorepo built for agent-assisted development. Keep the roo

## General Coding Rules

- `packages/agent-core-v2`, `packages/kap-server`, and `packages/transcript` are comment-free zones: no line/block comments; the exceptions are JSDoc attached to exported symbols and load-bearing lint-suppression directives (`oxlint-disable` / `eslint-disable`), while other tooling directives (`@ts-expect-error`, …) stay banned. Enforced by `scripts/check-no-comments.mjs`, which runs as part of `pnpm lint`.
- `packages/agent-core-v2`, `packages/kap-server`, and `packages/transcript` are comment-free zones: no comments of any kind — no line/block comments, no JSDoc (not even on exported symbols); the only exception is load-bearing lint-suppression directives (`oxlint-disable` / `eslint-disable`), while other tooling directives (`@ts-expect-error`, …) stay banned. Enforced by `scripts/check-no-comments.mjs` over `.ts`/`.tsx`/`.mts`/`.mjs` under `src/`/`test/`/`scripts/`, which runs as part of `pnpm lint`.
- For optional object properties, pass `undefined` directly instead of using conditional spread.
- YES: `{ user }`
- NO: `{ ...(user ? { user } : undefined) }`
Expand Down
2 changes: 1 addition & 1 deletion packages/agent-core-v2/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Domain-slice scenarios that used to live in `examples/<name>.example.ts` are now

## Comment conventions

- **No comments.** The code is the source of truth; do not write file headers, section banners, or implementation narration. The one exception is JSDoc attached to exported symbols (it flows into the generated `.d.ts` and the consumers' IDE hover); keep it focused on the public contract.
- **No comments.** The code is the source of truth; do not write file headers, section banners, implementation narration, or JSDoc — no comments of any kind, on exported symbols or not.
- **Lint-suppression directives are the tooling exception.** `oxlint-disable` / `eslint-disable` comments are allowed where they suppress an active rule for a deliberate pattern (e.g. the Event2 class+payload-interface merging idiom). `@ts-expect-error`, `@ts-ignore`, and `ts-nocheck` stay banned — fix the underlying type problem instead; negative type-safety cases go into compiler-asserted fixtures.

## Telemetry
Expand Down
143 changes: 0 additions & 143 deletions packages/agent-core-v2/scripts/check-import-boundaries.mjs
Original file line number Diff line number Diff line change
@@ -1,40 +1,4 @@
#!/usr/bin/env node
/**
* Import-boundary checker for `agent-core-v2`.
*
* Enforces two rules over `packages/agent-core-v2/src/**` (and the v1-import
* ban over `test/**` too):
*
* 1. **No v1 imports** — v2 must never `import '@moonshot-ai/agent-core'`
* (or any subpath). v2 ports logic; it never depends on v1.
* 2. **Kosong layering** — the `src/kosong/{contract,protocol,provider,model}`
* subtree has strict internal rules:
* - internal order: contract(L0) ← protocol(L1) ← provider/model(L2)
* ← catalog(L3); a lower layer never imports a higher one (so L1
* protocol never sees L2 — trait contexts carry only `providerId`).
* - peer rule: `model` may import `provider`, never the reverse.
* - purity: `contract` imports no other domain (only `_base` helpers)
* and no external package at all (no SDKs, not even types);
* `protocol` imports only `_base` + `contract` and no wire SDK.
* All pure layers may additionally import the DI vocabulary modules
* in `KOSONG_ALLOWED_VOCABULARY` (`app/scopes`).
* - `provider/bases/` sub-boundary: base implementation files must not
* import the registries (`protocolBase`, `protocolAdapterRegistry`),
* `providerDefinition`, or any `*.contrib.ts` module. The
* registration side lives in `*.contrib.ts` and in each base
* directory's `index.ts` barrel (import = registration); both are
* exempt.
* Kosong directories that do not exist yet are skipped silently (later
* refactor phases add them).
*
* Intra-package relative imports, `#/`-alias imports, and the package's
* self-reference (`@moonshot-ai/agent-core-v2/<path>` → `src/<path>`) are
* resolved against `src/`. Sibling packages (`@moonshot-ai/*` other than v1)
* and third-party imports are out of scope (except for the kosong purity
* bans above).
*
* Run: `node scripts/check-import-boundaries.mjs`. Exits non-zero on violation.
*/

import { readFileSync, readdirSync, statSync } from 'node:fs';
import { dirname, join, relative, resolve } from 'node:path';
Expand All @@ -48,65 +12,23 @@ const TEST_ROOT = join(PKG_ROOT, 'test');
const V1_PACKAGE = '@moonshot-ai/agent-core';
const SELF_PACKAGE_PREFIX = '@moonshot-ai/agent-core-v2/';

/**
* Scope directories introduced by the `src/{scope}/{domain}` layout. A path's
* first segment is a scope tier, not a domain; the domain is the next segment.
*/
const SCOPE_DIRS = new Set(['app', 'workspace', 'session', 'agent', 'persistence', 'os', 'kosong']);

/**
* Two-level scope directories: `persistence` and `os` use `{scope}/{tier}`
* (e.g. `persistence/interface`, `os/backends`) as the domain key; `kosong`
* uses `{scope}/{layer}` (e.g. `kosong/contract`) the same way.
*/
const TWO_LEVEL_SCOPES = new Set(['persistence', 'os', 'kosong']);

/**
* Kosong-internal layer order: contract ← protocol ← provider/model.
* A lower layer never imports a higher one; `model` → `provider`
* is the only allowed peer edge. Keyed by the segment under `src/kosong/`.
*/
const KOSONG_LAYER = new Map([
['contract', 0],
['protocol', 1],
['provider', 2],
['model', 2],
]);

/**
* Kosong is a pure provider/model abstraction layer: NO kosong subdomain may
* import another v2 domain outside kosong itself — only `_base` utilities
* are allowed, plus the DI vocabulary modules in
* `KOSONG_ALLOWED_VOCABULARY` (`app/scopes`: the `LifecycleScope` tier names
* every self-registering Service needs). (`protocol` additionally sees
* `kosong/contract`, handled by the internal-layer rule above.) Config
* persistence, OAuth tokens, events,
* and discovery orchestration all live in the upper `app/kosongConfig`
* wrapper — kosong must never reach up to them.
*/
const KOSONG_BASE_ONLY_SUBDOMAINS = new Set(['contract', 'protocol', 'provider', 'model']);

/**
* Non-`_base` modules the pure kosong layers may still import, keyed by
* extensionless `src/`-relative path. `app/scopes` is DI vocabulary (the
* scope tier names + topology declaration), not app orchestration, so a
* kosong Service may read its registration tier from it.
*/
const KOSONG_ALLOWED_VOCABULARY = new Set(['app/scopes']);

/**
* Wire SDK packages the pure kosong layers must never import — not even
* types. `contract` in fact imports no external package at all; this list
* covers the SDK ban for `protocol`.
*/
const KOSONG_BANNED_SDK_PACKAGES = ['@anthropic-ai/sdk', '@google/genai', 'openai'];

/**
* Parse an absolute path under `src/kosong/` into its subdomain info.
* Returns `undefined` for paths outside `src/kosong/`.
* @param {string} absPath
* @returns {{ sub: string | undefined, inBases: boolean, isContrib: boolean, isIndex: boolean } | undefined}
*/
function kosongInfoOf(absPath) {
const rel = relative(SRC_ROOT, absPath);
if (rel.startsWith('..') || rel === '') return undefined;
Expand All @@ -115,24 +37,13 @@ function kosongInfoOf(absPath) {
const sub = segments[1];
const last = segments[segments.length - 1] ?? '';
return {
// A file directly under `src/kosong/` has no subdomain.
sub: sub === undefined || sub.endsWith('.ts') ? undefined : sub,
inBases: sub === 'provider' && segments[2] === 'bases',
isContrib: last.endsWith('.contrib.ts'),
isIndex: last === 'index.ts',
};
}

/**
* Whether an import target is off-limits to base implementation files under
* `kosong/provider/bases/` (everything except `*.contrib.ts` and the
* registration `index.ts` barrels): the base registry
* (`kosong/protocol/protocolBase`), the adapter registry
* (`kosong/provider/protocolAdapterRegistry`), the provider-definition
* registry (`kosong/provider/providerDefinition`), or any contrib
* side-effect module. Matches extensionless specifiers too.
* @param {string} targetAbs
*/
function isKosongBasesBannedTarget(targetAbs) {
const rel = relative(SRC_ROOT, targetAbs).split(/[\\/]/).join('/');
const stripped = rel.endsWith('.ts') ? rel.slice(0, -'.ts'.length) : rel;
Expand All @@ -144,52 +55,30 @@ function isKosongBasesBannedTarget(targetAbs) {
);
}

/**
* Resolve a `src/`-relative path to its domain, skipping the scope tier when
* present. Returns `undefined` for top-level root files (e.g. the package
* barrel `index.ts`, or the `errors`/`hooks` facades).
* @param {string} rel
*/
function domainFromRel(rel) {
const segments = rel.split(/[\\/]/);
if (TWO_LEVEL_SCOPES.has(segments[0])) {
// `src/{persistence|os}/{interface|backends}/…`
return segments[1] ? `${segments[0]}/${segments[1]}` : segments[0];
}
if (SCOPE_DIRS.has(segments[0])) {
if (segments.length === 2 && segments[1]?.endsWith('.ts')) return segments[0];
// `src/{scope}/{domain}/…`
if (segments[0] === 'agent' && segments[1] === 'task') return 'agentTask';
if (segments[0] === 'agent' && segments[1] === 'plugin') return 'agentPlugin';
return segments[1];
}
return segments[0];
}

/**
* Determine the v2 domain for an *import target* absolute path. A target may
* resolve straight to a domain directory — e.g. the bare domain import
* `#/turn` resolves to `src/agent/turn`, whose domain is `turn`.
* @param {string} targetAbs
*/
function targetDomainOf(targetAbs) {
const rel = relative(SRC_ROOT, targetAbs);
if (rel.startsWith('..') || rel === '') return undefined;
return domainFromRel(rel);
}

/**
* Resolve an import specifier to an absolute v2 `src/` path, or `undefined`
* when the specifier is not an intra-v2 import.
* @param {string} specifier
* @param {string} fromFile absolute path of the importing file
*/
function resolveIntraV2(specifier, fromFile) {
if (specifier.startsWith('#/')) {
return join(SRC_ROOT, specifier.slice(2));
}
// The package's legal self-reference: `@moonshot-ai/agent-core-v2/x` maps
// to `src/x` via the `./*` export.
if (specifier.startsWith(SELF_PACKAGE_PREFIX)) {
return join(SRC_ROOT, specifier.slice(SELF_PACKAGE_PREFIX.length));
}
Expand All @@ -199,22 +88,9 @@ function resolveIntraV2(specifier, fromFile) {
return undefined;
}

// Matches: import ... from 'x' | export ... from 'x' | import('x') | require('x')
const IMPORT_RE =
/(?:import|export)\s+(?:type\s+)?(?:[^'";]*?\s+from\s+)?['"]([^'"]+)['"]|(?:import|require)\s*\(\s*['"]([^'"]+)['"]\s*\)/g;

/**
* @typedef {{ file: string, line: number, message: string }} Violation
*/

/**
* Check source text for boundary violations. `absFile` is used only to
* resolve relative specifiers and determine the source location; the file
* need not exist on disk (handy for tests).
* @param {string} source
* @param {string} absFile
* @returns {Violation[]}
*/
export function checkSource(source, absFile) {
const violations = [];
const inSrc = !relative(SRC_ROOT, absFile).startsWith('..');
Expand All @@ -226,7 +102,6 @@ export function checkSource(source, absFile) {
if (!specifier) continue;
const line = source.slice(0, match.index).split('\n').length;

// Rule 1: v2 must not import v1.
if (specifier === V1_PACKAGE || specifier.startsWith(`${V1_PACKAGE}/`)) {
violations.push({
file: absFile,
Expand All @@ -236,15 +111,11 @@ export function checkSource(source, absFile) {
continue;
}

// Rule 2: kosong subtree (production code only).
if (!inSrc) continue;
const targetAbs = resolveIntraV2(specifier, absFile);
const sourceKosong = kosongInfoOf(absFile);
if (sourceKosong === undefined) continue;

// Rule 2a: kosong purity bans on external packages. The L0 contract
// imports no external package at all (no SDKs, not even types); the L1
// protocol layer is SDK-free but may use general-purpose packages.
if (targetAbs === undefined) {
if (sourceKosong.sub === 'contract') {
violations.push({
Expand All @@ -267,9 +138,6 @@ export function checkSource(source, absFile) {
continue;
}

// Rule 2b: kosong-internal layering. Runs even for same-domain imports
// because the provider/bases sub-boundary also bans same-domain targets
// (registries and contrib modules live beside the bases).
const targetKosong = kosongInfoOf(targetAbs);
if (targetKosong !== undefined) {
const sourceKosongLayer = KOSONG_LAYER.get(sourceKosong.sub);
Expand Down Expand Up @@ -304,11 +172,6 @@ export function checkSource(source, absFile) {
continue;
}

// Rule 2c: outside the kosong subtree, kosong code may only depend on
// `_base` utilities plus the DI vocabulary in KOSONG_ALLOWED_VOCABULARY
// (`protocol` additionally sees `kosong/contract`,
// handled by Rule 2b above). This is what keeps kosong a pure
// abstraction layer with no upward dependencies.
if (KOSONG_BASE_ONLY_SUBDOMAINS.has(sourceKosong.sub)) {
const targetDomain = targetDomainOf(targetAbs);
const targetRel = relative(SRC_ROOT, targetAbs).split(/[\\/]/).join('/');
Expand All @@ -326,17 +189,11 @@ export function checkSource(source, absFile) {
return violations;
}

/**
* Check a single source file for boundary violations.
* @param {string} absFile
* @returns {Violation[]}
*/
export function checkFile(absFile) {
return checkSource(readFileSync(absFile, 'utf8'), absFile);
}

function walk(dir) {
/** @type {string[]} */
const out = [];
for (const entry of readdirSync(dir)) {
if (entry === 'node_modules' || entry === 'dist') continue;
Expand Down
Loading
Loading