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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion .agents/skills/agent-core-dev/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ End-to-end procedures that span the stages. Reach for these before reading the s

## Stages

- [Stage 1 — Orient](orient.md): the DI black box (identity / dependencies / lifetime), the four `LifecycleScope` tiers and visibility, and the file-header comment convention. Read before touching business code.
- [Stage 1 — Orient](orient.md): the DI black box (identity / dependencies / lifetime), the four `LifecycleScope` tiers and visibility, and the no-comment convention. Read before touching business code.
- [Stage 2 — Design a service](design.md): pick a scope, split a domain across scopes, choose a calling style (direct call vs event vs hook), and direct dependencies. Decide *where things live and who knows whom* before coding.
- Topic: [Domain boundaries vs Scope](domain-boundaries.md) — keep `session` / `agent` / `turn` from becoming god objects; data-ownership test and their split conclusions.
- Topic: [Persistence layering](persistence.md) — the three-layer `Store → Storage → backend` model, naming Stores by access pattern, and which layer business code should depend on.
Expand Down
39 changes: 3 additions & 36 deletions .agents/skills/agent-core-dev/orient.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,45 +66,12 @@ There is no domain-layer numbering — a domain may import any other domain, gui
- v2 never imports v1 (`@moonshot-ai/agent-core` or any subpath).
- The kosong subtree (`src/kosong/{contract,protocol,provider,model}`) keeps its strict internal order (`contract ← protocol ← provider/model`), purity bans (no SDKs in `contract`/`protocol`), and the `provider/bases` registration boundary.

## File-header comment convention
## Comment convention

`packages/agent-core-v2/AGENTS.md` mandates a header-only comment style:

- **Header only.** Comments live solely in the top-of-file `/** */` block — never beside functions, methods, or statements. The code is the source of truth for *how*; the header states *what the module exposes and the responsibility it owns*.
- **Identity line first.** Start with `` `<domain>` domain — <one-line role>. `` Keep an existing `(cross-cutting)` label as-is. Write the role as a responsibility ("drives the turn lifecycle"), not a symbol list.
- **Scope is in the filename.** `workspace*.ts` = Workspace, `session*.ts` = Session, `agent*.ts` = Agent, no prefix = App (see service-authoring.md). State the same scope in the header so the two never drift.
- **Interface files** (`<name>.ts`) state the public contract + scope: which `IXxx` they define and what it is for.
- **Impl files** (`<name>Service.ts`) add collaborators + scope: list every imported cross-domain collaborator as a role ("persists records through `records`"); read scope from `registerScopedService(LifecycleScope.X, …)`.
- **Contribution files** (`<targetDomain>.ts` / `<what>.contrib.ts`) state what they register into the target domain (e.g. "registers the `log` config section into `config`").
- **Pure-function / `.types` / `.errors` files** state the responsibility only — they own no scoped state, so no scope line.

Impl file example (`sessionMetadataService.ts`):

```ts
/**
* `sessionMetadata` domain — `ISessionMetadata` implementation.
*
* Persists the session metadata document (`state.json`) through the `storage`
* access-pattern store (`IAtomicDocumentStore`), rooted at the `metaScope`
* namespace from `sessionContext`. Loads the existing document on
* construction (creating it on first run), and logs through `log`. Bound at
* Session scope.
*/
```

Contribution file example (`config.ts` inside `log/`):

```ts
/**
* `log` domain — registers the `log` config section into `config`.
*
* Owns the `log` section schema and its env overlay; imported for the
* registration side effect. Bound at App scope.
*/
```
`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).

## 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.
- File-header comments describe role and scope only; never narrate implementation beside statements.
- No comments — not file headers, not beside statements; exported-symbol JSDoc is the only exception.
4 changes: 2 additions & 2 deletions .agents/skills/agent-core-dev/server-align.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ registerScopedService(
Conventions:

- **Name** the domain `<domain>Legacy` and the interface with the scope prefix, `I<Scope><Domain>LegacyService` (e.g. `prompt` / `IAgentPromptService`), per service-authoring.md.
- **Header comment** must say it is an `edge adapter` and name both the v1 contract it implements and the native v2 Service it leaves untouched (see `prompt.ts`).
- **Role is carried by the name** — `<domain>Legacy` marks it as an `edge adapter`; the v1 contract it implements and the native v2 Service it leaves untouched stay evident from its delegation targets (see `prompt.ts`).
- **Scope** = the lifetime of the *legacy* state it holds (the `prompt` queue is per-agent → `LifecycleScope.Agent`). Apply [orient.md](orient.md) / [design.md](design.md) normally — a LegacyService is not exempt from scope rules.
- **Delegate, do not duplicate** business logic. The LegacyService translates the v1 contract into native-Service calls and translates results back; the real work stays in the native Service.
- **Contract types come from the v1 wire schema homes** (the owning v2 domain contract or `kap-server/src/protocol`), so the interface cannot drift from the wire shape.
Expand Down Expand Up @@ -236,7 +236,7 @@ Before submitting a server-align change:
- [ ] Request and response schemas come from their owning home (the `agent-core-v2` domain contract or `packages/kap-server/src/protocol`); no inline re-declaration in server-v2.
- [ ] Existing schema fields are unchanged in name, type, and semantics; only optional fields added (if any).
- [ ] Native v2 Service left clean; v1-only behavior isolated in a `<domain>Legacy` / `I<Domain>LegacyService` edge adapter when the semantics diverge.
- [ ] LegacyService registered with the correct `LifecycleScope` and a header comment naming it an edge adapter + the native Service it preserves.
- [ ] LegacyService registered with the correct `LifecycleScope` and named as the `<domain>Legacy` edge adapter preserving the native Service.
- [ ] Domain error codes registered in `agent-core-v2`; wire codes registered in `packages/kap-server/src/protocol`; route maps them in `sendMappedError`, matching v1's status codes and idempotent envelopes.
- [ ] Route resolves the scope from the URL by `accessor.get(IX)`; no cached scope; finishes before disposal.
- [ ] Tests assert the wire envelope + protocol shape; wire-shape guards added/updated where the route mirrors v1.
Expand Down
7 changes: 3 additions & 4 deletions .agents/skills/agent-core-dev/service-authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ One folder per domain, **camelCase**: `session/`, `sessionActivity/`, `contextMe
```

- **Strictly one service per file.** An interface file holds exactly one injectable interface and exactly one `createDecorator(...)`; an impl file holds exactly one service implementation class and exactly one `registerScopedService(...)`. No exceptions for "tightly-coupled" groups: even same-scope collaborators each get their own `<name>.ts` + `<name>Service.ts` pair.
- **Scope is in the filename.** `workspace*.ts` = Workspace, `session*.ts` = Session, `agent*.ts` = Agent, no scope prefix = App (see [Naming](#naming)). The header comment restates the same scope.
- **Scope is in the filename.** `workspace*.ts` = Workspace, `session*.ts` = Session, `agent*.ts` = Agent, no scope prefix = App (see [Naming](#naming)).
- A domain therefore has as many impl files as it has services (e.g. `logService.ts` for the App `ILogService`, `sessionLogService.ts` for the Session `ISessionLogService`). See [Multi-Service domains](#multi-service-domains).

The package entry `src/index.ts` imports and `export *`s every domain's leaf files precisely (one line per leaf), so importing the package still runs every `registerScopedService(...)` side effect — exactly as the old per-domain barrels did.
Expand Down Expand Up @@ -293,11 +293,10 @@ Importing the package therefore fires every `register*` side effect, exactly as

- Load the impl file too — its top-level `registerScopedService(...)` only runs when the module is imported.
- `export *` helper modules only if they are part of the domain's public surface.
- Each leaf's file-header comment still names the domain, scope, and (for impls) the `register*` binding it owns.

## Comments

- **File-header comment is mandatory** and the only place comments live (orient.md). State the identity line, the role, collaborators (impls), and scope.
- **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.
- For unimplemented stubs, throw `NotImplementedError('feature')` rather than `throw new Error('TODO: …')` (errors.md).
Expand Down Expand Up @@ -351,4 +350,4 @@ import './greet/greetService';
- Never `new` a `@IService`-carrying Service — except inside an explicit factory method, which is not a DI request.
- Events: typed per-Service event → `Event<T>`/`Emitter` from `'#/_base/event'`; cross-domain broadcast → `IEventService` from `'#/event'`.
- `src/index.ts` must import/export every leaf file (including the impl) so each `register*` side effect runs.
- File-header comment only; methods/fields carry no comments by default; stubs throw `NotImplementedError`.
- No comments by default (orient.md); stubs throw `NotImplementedError`.
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** — header comments describe role + scope only; registration runs from the impl file's top level; the new domain is exported from `src/index.ts`.
- **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`.

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
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +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`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Align the test skills with the new comment ban

When an agent follows the repository's test guidance for an agent-core-v2 test, the new rule makes that guidance fail lint: .agents/skills/agent-core-review/test/SKILL.md:91 mandates a per-file header comment, and .agents/skills/agent-core-dev/test.md:24 requires an explanatory comment for a supported two-instance test pattern. Update those instructions to comment-free alternatives as part of introducing this ban, otherwise the repository gives mutually incompatible mandatory workflows.

AGENTS.md reference: AGENTS.md:L49-L51

Useful? React with 👍 / 👎.

- 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 package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"vis": "pnpm -C apps/vis run dev",
"dev:docs": "pnpm -C docs install --ignore-workspace && pnpm -C docs run dev",
"typecheck": "pnpm run build:packages && pnpm -r --filter './packages/*' run typecheck && pnpm --filter @moonshot-ai/kimi-code run typecheck && pnpm --filter kimi-code run typecheck && pnpm --filter @moonshot-ai/vis-server run typecheck && pnpm --filter @moonshot-ai/vis-web run typecheck",
"lint": "oxlint --type-aware",
"lint": "node scripts/check-no-comments.mjs && oxlint --type-aware",
"lint:fix": "pnpm run lint --fix",
"lint:pkg": "pnpm --filter @moonshot-ai/kimi-code exec publint && npm_config_cache=${TMPDIR:-/tmp}/kimi-code-npm-cache pnpm --filter @moonshot-ai/kimi-code exec attw --pack . --profile node16",
"sherif": "sherif -i @agentclientprotocol/sdk",
Expand Down
23 changes: 3 additions & 20 deletions packages/agent-core-v2/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Four `LifecycleScope` tiers — `App` / `Workspace` / `Session` / `Agent` (strin

The DI kernel (`src/_base/di/`) owns the unit layer on top of the scoped registry:

- `service.ts` — `Service`: the unit base class (extends `Disposable`). Capabilities live on `this` (`provide` / `effect` / `on` / `get` / `ref`, plus `name` / `state` / `config`). Two-phase construction: inside the ctor `provide`/`on`/`effect` buffer (writes only — `get`/`ref` throw, dependencies are constructor parameters); the kernel binds the runtime after `Reflect.construct` and flushes in writing order; a manually `new`ed instance throws on every capability call. Services whose own members collide with the `Service` vocabulary keep `extends Disposable` with a NOTE comment — still full DI units (cascade/ledger do not require `Service`).
- `service.ts` — `Service`: the unit base class (extends `Disposable`). Capabilities live on `this` (`provide` / `effect` / `on` / `get` / `ref`, plus `name` / `state` / `config`). Two-phase construction: inside the ctor `provide`/`on`/`effect` buffer (writes only — `get`/`ref` throw, dependencies are constructor parameters); the kernel binds the runtime after `Reflect.construct` and flushes in writing order; a manually `new`ed instance throws on every capability call. Services whose own members collide with the `Service` vocabulary keep `extends Disposable` — still full DI units (cascade/ledger do not require `Service`).
- `fiber.ts` — the `Fiber` capability interface (not a DI token), `FiberHandle` (thenable / `state` / `uid` / `update` / `dispose`), `ServiceRecipe` (class / arrow function / `{apply}`), the `FiberState` five-state machine, and `ScopeUnits(kind)` — the materialization collection token, one per scope kind.
- `collection.ts` — `collection<T>(name)` contribution tokens. Contribute with `this.provide(token, value)`; a fold declares the token as a constructor parameter and receives a `CollectionView<T>` (`items` / `records` / incremental `onDidChange`). Records are visible to the provider's ancestors and descendants (never sibling subtrees); provider death withdraws. Collection edges enter the graph for introspection but never join a cascade contagion set.
- `scopeUnits.ts` — the kernel fold: every scope-creation point (`createScopedChildHandle` / `Scope.createApp` / `Scope.createChild`) runs `watchScopeUnits(container, kind)` before eager activation, materializing each visible `ScopeUnits(kind)` record's recipe as a unit inside the new scope (disposal hangs on the record provider's book — provider death tears the materialized units down across the tree). `ScopeOptions.configureContainer` runs at the same point (the session seed adapters use it).
Expand All @@ -35,25 +35,8 @@ Domain-slice scenarios that used to live in `examples/<name>.example.ts` are now

## Comment conventions

- **Header only, external role only.** Comments live solely in the top-of-file `/** */` block — never beside functions, methods, or statements. Say what the module exposes and the responsibility it owns; the code is the source of truth for how it works, so do not narrate implementation steps, enumerate every export, or note porting / skeleton status.
- **Identity line first.** Start with `` `<domain>` domain — <one-line role>. `` Keep an existing `(cross-cutting)` label as-is. Write the role as a responsibility ("drives the turn lifecycle"), not a symbol list ("turn driver + context + loop runner").
- **Impl files add collaborators + scope; contract files add the public contract + scope.** For impls, list every imported cross-domain collaborator as a role ("persists records through `records`") — declared dependencies count even if not yet wired in this WIP port; infrastructure imports (`_base/**`) are not collaborators. Read scope from `registerScopedService(LifecycleScope.X, …)`.

### Examples

Impl (`src/session/sessionMetadata/sessionMetadataService.ts`):

```ts
/**
* `sessionMetadata` domain — `ISessionMetadata` implementation.
*
* Persists the session metadata document (`state.json`) through the `storage`
* access-pattern store (`IAtomicDocumentStore`), rooted at the `metaScope`
* namespace from `sessionContext`. Loads the existing document on
* construction (creating it on first run), and logs through `log`. Bound at
* Session scope.
*/
```
- **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.
- **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
2 changes: 1 addition & 1 deletion packages/agent-core-v2/docs/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ keep their static registrations; the service and the two tools go through the Fe

## Adding a new feature

1. `src/features/<name>/` — domain files follow the usual conventions (header comments,
1. `src/features/<name>/` — domain files follow the usual conventions (no comments,
one service per file pair, `.md?raw` assets move with the feature).
2. `<name>Feature.ts` — the Feature subclass + `registerFeature(...)`.
3. `src/index.ts` — precise leaf imports/exports; no barrel.
Expand Down
Loading
Loading