From b94ca5232fc9ac0c745684aaff6df3cf38119a88 Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Sat, 30 May 2026 18:00:13 +0700 Subject: [PATCH] feat: [ENG-2929] add brv channel core interfaces and enforce server/core import boundary. --- CLAUDE.md | 4 + eslint.config.mjs | 29 +++++++ .../domain/render/curate-prompt-builder.ts | 4 + .../core/interfaces/channel/i-agent-driver.ts | 84 +++++++++++++++++++ .../channel/i-channel-broadcaster.ts | 22 +++++ .../channel/i-channel-orchestrator.ts | 58 +++++++++++++ .../interfaces/channel/i-channel-store.ts | 63 ++++++++++++++ .../core/interfaces/channel/i-driver-pool.ts | 46 ++++++++++ .../interfaces/channel/i-transcript-store.ts | 38 +++++++++ .../interfaces/executor/i-curate-executor.ts | 2 + src/shared/types/channel.ts | 14 ++++ 11 files changed, 364 insertions(+) create mode 100644 src/server/core/interfaces/channel/i-agent-driver.ts create mode 100644 src/server/core/interfaces/channel/i-channel-broadcaster.ts create mode 100644 src/server/core/interfaces/channel/i-channel-orchestrator.ts create mode 100644 src/server/core/interfaces/channel/i-channel-store.ts create mode 100644 src/server/core/interfaces/channel/i-driver-pool.ts create mode 100644 src/server/core/interfaces/channel/i-transcript-store.ts diff --git a/CLAUDE.md b/CLAUDE.md index 8cdb380e9..21bfc3300 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,6 +36,10 @@ npm run dev:ui:package # Vite dev server resolving shared UI from - Use `??` for nullish defaults (not `||`, which also triggers on `0`/`''`/`false`) and `?.` for safe property access - Prefer optional properties (`foo?: T`) over `foo: T | undefined` when a key may legitimately be absent +**Comments / JSDoc**: +- Doc comments for functions and methods start with a third-person singular present-tense verb (`Creates a channel`, `Returns the driver`, `Appends an event`), not the imperative (`Create`, `Return`, `Append`). This applies to method/function descriptions; type, property, and parameter docs are not constrained this way. +- Don't embed ephemeral project-planning references in code/doc comments — milestone labels (`M1`, `M5+`), open-question numbers (`Q7`), or Linear issue IDs. They rot as plans change and mislead later readers. State the durable *why* (e.g. "a local subprocess and a remote peer implement the same interface"), not the *when* (which milestone). Exception: a `TODO(ENG-1234)` pointing at a tracked follow-up issue is the correct way to mark known debt. Milestone/issue-linked rationale belongs in Linear and the byterover context tree, not in code. + **Testing (Strict TDD — MANDATORY)**: - You MUST follow Test-Driven Development. This is non-negotiable. - **Step 1 — Write failing tests FIRST**: Before writing ANY implementation code, write or update tests that describe the expected behavior. Do NOT write implementation and tests together or in reverse order. diff --git a/eslint.config.mjs b/eslint.config.mjs index dd28e7577..026c49b2a 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -88,4 +88,33 @@ export default [ ], }, }, + // Architecture boundary: src/server/core may depend only on abstractions. + // NOTE: infra is a SIBLING of core under src/server/, so a core→infra relative import + // (e.g. ../../../infra/foo.js) has NO "server" segment — the sibling-relative ../infra + // variants below are REQUIRED in addition to the **/server/infra/** form. + { + files: ['src/server/core/**/*.ts'], + rules: { + 'no-restricted-imports': [ + 'error', + { + patterns: [ + { + group: ['**/server/infra/**', '../infra/**', '../../infra/**', '../../../infra/**', '../../../../infra/**'], + message: + 'core must not import from server/infra. Depend on an interface in core/interfaces and let infra implement it (dependency inversion).', + }, + { + group: ['**/oclif/**', '../oclif/**', '../../oclif/**', '../../../oclif/**', '../../../../oclif/**'], + message: 'core must not import from oclif. Keep CLI wiring out of the domain/application core.', + }, + { + group: ['**/tui/**', '../tui/**', '../../tui/**', '../../../tui/**', '../../../../tui/**'], + message: 'core must not import from tui. Keep UI out of the domain/application core.', + }, + ], + }, + ], + }, + }, ] diff --git a/src/server/core/domain/render/curate-prompt-builder.ts b/src/server/core/domain/render/curate-prompt-builder.ts index ea0a7ef52..d684bd5ec 100644 --- a/src/server/core/domain/render/curate-prompt-builder.ts +++ b/src/server/core/domain/render/curate-prompt-builder.ts @@ -1,5 +1,9 @@ +// TODO(ENG-3034): relocate the HtmlWriteError type into core so this stops importing infra. +// eslint-disable-next-line no-restricted-imports import type {HtmlWriteError} from '../../../infra/render/writer/html-writer.js' +// TODO(ENG-3034): inject ELEMENT_REGISTRY (or move it into core) so this stops importing infra. +// eslint-disable-next-line no-restricted-imports import {ELEMENT_REGISTRY} from '../../../infra/render/elements/registry.js' import {ELEMENT_NAMES} from './element-types.js' diff --git a/src/server/core/interfaces/channel/i-agent-driver.ts b/src/server/core/interfaces/channel/i-agent-driver.ts new file mode 100644 index 000000000..a57e4b184 --- /dev/null +++ b/src/server/core/interfaces/channel/i-agent-driver.ts @@ -0,0 +1,84 @@ +import type {ContentBlock, TurnEvent} from '../../../../shared/types/index.js' + +/** + * Payload-only `TurnEvent`: a variant's fields WITHOUT the base coordination + * metadata (`channelId`, `turnId`, `deliveryId`, `memberHandle`, `emittedAt`, + * `seq`). A driver is oblivious to channel-side coordinates — the orchestrator + * stamps the base fields (including the gap-free monotonic `seq`) as it relays + * each payload into the transcript, so correct ordering can only be assigned by + * that single writer. + * + * Derived structurally from {@link TurnEvent} via a distributive conditional so + * the two can never drift: adding a new `TurnEvent` variant updates this type + * automatically. The omitted keys MUST mirror the `TurnEvent` base shape. + */ +export type TurnEventPayload = TurnEvent extends infer T + ? T extends TurnEvent + ? Omit + : never + : never + +/** Arguments for a single prompt/turn dispatched to a driver. */ +export type AgentDriverPromptArgs = { + /** Opaque per-turn metadata forwarded to the underlying agent (driver-defined). */ + readonly meta?: Record + /** Prompt content blocks for this turn. */ + readonly prompt: ContentBlock[] + /** Channel turn this dispatch belongs to (correlation only; not echoed in payloads). */ + readonly turnId: string +} + +/** Lifecycle status of a single driver instance. */ +export type AgentDriverStatus = 'errored' | 'idle' | 'stopped' | 'streaming' + +/** + * Transport-agnostic contract for driving one agent and streaming its turn — the + * single most important seam in the channel subsystem. A local ACP subprocess + * and a remote A2A peer implement THIS SAME interface, so the orchestrator never + * knows or cares whether an agent is local or networked. + * + * Deliberately free of ACP vocabulary: no protocol version, no ACP capability + * snapshot, no ACP `initialize` handshake — those belong to the concrete ACP + * implementation, not to this contract. + * + * One instance serves one channel member. Spawn / teardown is the caller's + * concern via {@link IAgentDriver.start} / {@link IAgentDriver.stop}. + */ +export interface IAgentDriver { + /** + * Cancels in-flight work. With a `turnId`, cancels just that turn; without, it + * cancels whatever is currently streaming. Idempotent; later prompts still work. + */ + cancel(turnId?: string): Promise + + /** Stable channel-member handle this driver serves (e.g. `@claude`). */ + readonly handle: string + + /** + * Dispatches a prompt and stream the turn as it unfolds. Each yielded + * {@link TurnEventPayload} is a base-field-free slice; the orchestrator stamps + * `channelId` / `turnId` / `deliveryId` / `memberHandle` / `seq` / `emittedAt` + * before persisting and broadcasting. The iterator completes when the turn + * reaches a terminal state and may throw to signal a driver-level failure. + */ + prompt(args: AgentDriverPromptArgs): AsyncIterableIterator + + /** + * Resolves a pending permission request the driver surfaced (via a + * `permission_request` payload). `response` is opaque here; the concrete driver + * interprets it. + * + * @param permissionRequestId - Id from the `permission_request` payload. + * @param response - Driver-defined decision payload. + */ + respondToPermission(permissionRequestId: string, response: unknown): Promise + + /** Brings the underlying session up (spawn / connect / handshake). Idempotent. */ + start(): Promise + + /** Current lifecycle status. */ + readonly status: AgentDriverStatus + + /** Tears the session down and release resources. Idempotent. */ + stop(): Promise +} diff --git a/src/server/core/interfaces/channel/i-channel-broadcaster.ts b/src/server/core/interfaces/channel/i-channel-broadcaster.ts new file mode 100644 index 000000000..4d410b913 --- /dev/null +++ b/src/server/core/interfaces/channel/i-channel-broadcaster.ts @@ -0,0 +1,22 @@ +/** + * Outbound fan-out port used by the channel orchestrator. + * + * The orchestrator lives in the core layer and MUST NOT depend on the transport + * server directly — the transport may later be swapped for a cross-machine + * relay. The infra adapter binds this port to the real transport + * server, delegating to `broadcastTo('channel:', event, data)`. + * + * Fire-and-forget: there is no awaitable delivery guarantee. Subscribers are the + * clients (TUI / webui / cli) joined to the `channel:` room. + */ +export interface IChannelBroadcaster { + /** + * Emits `event` (with payload `data`) to every client subscribed to + * `channel:`. + * + * @param channelId - Channel whose subscribers receive the event. + * @param event - Transport event name (e.g. `channel:turn-event`). + * @param data - Event payload; shape is event-specific. + */ + broadcastToChannel(channelId: string, event: string, data: T): void +} diff --git a/src/server/core/interfaces/channel/i-channel-orchestrator.ts b/src/server/core/interfaces/channel/i-channel-orchestrator.ts new file mode 100644 index 000000000..75225cca7 --- /dev/null +++ b/src/server/core/interfaces/channel/i-channel-orchestrator.ts @@ -0,0 +1,58 @@ +import type {Channel, ContentBlock, Turn} from '../../../../shared/types/index.js' + +/** Cancel an in-flight turn. */ +export type CancelTurnArgs = { + readonly channelId: string + readonly turnId: string +} + +/** Create a new channel. */ +export type CreateChannelArgs = { + readonly channelId: string + readonly title?: string +} + +/** Post a new turn (a user or local-agent prompt) into a channel. */ +export type PostTurnArgs = { + readonly channelId: string + /** Optional client-supplied idempotency key for safe retries. */ + readonly idempotencyKey?: string + /** Handles explicitly mentioned in the prompt; drives dispatch. */ + readonly mentions?: string[] + readonly promptBlocks: ContentBlock[] +} + +/** + * Thin application-facing coordinator for the channel subsystem. Deliberately + * smaller than the POC's god-object: it exposes only the read + lifecycle + * operations the foundation needs. The interface is EXTENDED (never replaced) as + * the subsystem grows — member management, streaming dispatch, quorum fan-out, + * and permission decisions land additively once their domain types and consumers + * exist. + * + * Implementations validate inputs against the transport request schemas before + * these methods run (the handler does this), so orchestrator methods can trust + * their arguments. + */ +export interface IChannelOrchestrator { + /** Cancels an in-flight turn and its deliveries. */ + cancelTurn(args: CancelTurnArgs): Promise + + /** Creates a new channel and returns its record. */ + createChannel(args: CreateChannelArgs): Promise + + /** Reads one channel, or `undefined` when it does not exist. */ + getChannel(channelId: string): Promise + + /** Reads one turn within a channel, or `undefined` when it does not exist. */ + getTurn(channelId: string, turnId: string): Promise + + /** Lists all channels. */ + listChannels(): Promise + + /** Lists the turns of a channel; ordering is defined by the adapter. */ + listTurns(channelId: string): Promise + + /** Posts a new turn into a channel and returns the created turn record. */ + postTurn(args: PostTurnArgs): Promise +} diff --git a/src/server/core/interfaces/channel/i-channel-store.ts b/src/server/core/interfaces/channel/i-channel-store.ts new file mode 100644 index 000000000..955af0f72 --- /dev/null +++ b/src/server/core/interfaces/channel/i-channel-store.ts @@ -0,0 +1,63 @@ +import type {Channel, ChannelMember, ChannelSettings} from '../../../../shared/types/index.js' + +/** Add a full member record to a channel. */ +export type ChannelStoreAddMemberArgs = { + readonly channelId: string + readonly member: ChannelMember +} + +/** Create a new channel record. */ +export type ChannelStoreCreateArgs = { + readonly channelId: string + readonly settings?: ChannelSettings + readonly title?: string +} + +/** Remove a member from a channel by handle. */ +export type ChannelStoreRemoveMemberArgs = { + readonly channelId: string + readonly memberHandle: string +} + +/** Apply a metadata patch to an existing channel (title / settings / archive). */ +export type ChannelStoreUpdateArgs = { + readonly archivedAt?: string + readonly channelId: string + readonly settings?: ChannelSettings + readonly title?: string +} + +/** + * Persistence port for channel + member METADATA only. It owns the durable + * {@link Channel} record and the full {@link ChannelMember} records behind it; + * `Channel.members` remains the summarised projection the adapter derives from + * those records. + * + * It deliberately does NOT store transcripts (turns / events) — that is + * `ITranscriptStore`'s responsibility. Splitting the two lets transcript + * retention / GC evolve independently, without touching channel metadata. Member + * CRUD (`addMember` / `removeMember` / `listMembers`) is the seam invite / + * uninvite drives. + */ +export interface IChannelStore { + /** Persists a new member record under a channel. */ + addMember(args: ChannelStoreAddMemberArgs): Promise + + /** Creates and persists a new channel; rejects when `channelId` already exists. */ + createChannel(args: ChannelStoreCreateArgs): Promise + + /** Lists all channels (summary view). */ + listChannels(): Promise + + /** Lists the full member records of a channel. */ + listMembers(channelId: string): Promise + + /** Reads one channel record, or `undefined` when it does not exist. */ + readChannel(channelId: string): Promise + + /** Removes a member record from a channel by handle. No-op when absent. */ + removeMember(args: ChannelStoreRemoveMemberArgs): Promise + + /** Applies a metadata patch to an existing channel; returns the updated record. */ + updateChannel(args: ChannelStoreUpdateArgs): Promise +} diff --git a/src/server/core/interfaces/channel/i-driver-pool.ts b/src/server/core/interfaces/channel/i-driver-pool.ts new file mode 100644 index 000000000..6894e6060 --- /dev/null +++ b/src/server/core/interfaces/channel/i-driver-pool.ts @@ -0,0 +1,46 @@ +import type {IAgentDriver} from './i-agent-driver.js' + +/** Composite key identifying one member's driver slot within a channel. */ +export type DriverPoolKey = { + readonly channelId: string + readonly memberHandle: string +} + +/** Look up the driver registered for a `{channelId, memberHandle}`. */ +export type DriverPoolAcquireArgs = DriverPoolKey + +/** Register an already-started driver under its `{channelId, memberHandle}` key. */ +export type DriverPoolRegisterArgs = DriverPoolKey & { + readonly driver: IAgentDriver +} + +/** Release (stop + evict) the driver for a `{channelId, memberHandle}`. */ +export type DriverPoolReleaseArgs = DriverPoolKey + +/** + * Keyed registry of live {@link IAgentDriver} instances — one slot per + * `{channelId, memberHandle}`. The pool is pure lifecycle bookkeeping: it does + * NOT spawn drivers. The orchestrator constructs + starts a driver and hands it + * over via {@link IDriverPool.register}; `acquire` is a non-blocking lookup; the + * `release*` methods call `driver.stop()` so subprocess agents never leak. + * + * Pre-warming is intentionally absent: it has no consumer at this layer yet, and + * the pool's "never constructs drivers" invariant means warming belongs to the + * orchestrator (which owns driver construction) when a consumer needs it. + */ +export interface IDriverPool { + /** Returns the registered driver for the key, or `undefined` if none. Never spawns. */ + acquire(args: DriverPoolAcquireArgs): IAgentDriver | undefined + + /** Stores an already-started driver under its key, replacing any prior slot. */ + register(args: DriverPoolRegisterArgs): void + + /** Stops and evict the driver for a single key. No-op when absent. */ + release(args: DriverPoolReleaseArgs): Promise + + /** Stops and evict every driver in the pool (daemon shutdown). */ + releaseAll(): Promise + + /** Stops and evict all drivers belonging to a channel (channel close / archive). */ + releaseChannel(channelId: string): Promise +} diff --git a/src/server/core/interfaces/channel/i-transcript-store.ts b/src/server/core/interfaces/channel/i-transcript-store.ts new file mode 100644 index 000000000..80a025810 --- /dev/null +++ b/src/server/core/interfaces/channel/i-transcript-store.ts @@ -0,0 +1,38 @@ +import type {TurnEvent} from '../../../../shared/types/index.js' + +/** + * Append one fully-stamped transcript event. Location-agnostic by design: the + * caller supplies `projectRoot` + `channelId` + `turnId`; the contract says + * nothing about files, NDJSON, per-turn indexes, or whether storage is + * per-project vs global. The path / retention policy is the adapter's concern. + */ +export type AppendTurnEventArgs = { + readonly channelId: string + /** Fully-stamped event — base fields already populated by the orchestrator. */ + readonly event: TurnEvent + /** Project the channel lives under; the adapter resolves storage location from it. */ + readonly projectRoot: string + readonly turnId: string +} + +/** Read back every persisted event for one turn, in `seq` order. */ +export type ReadTurnEventsArgs = { + readonly channelId: string + readonly projectRoot: string + readonly turnId: string +} + +/** + * Append-and-read port for a turn's event log. Deliberately minimal: it does NOT + * model turn / channel metadata (`IChannelStore` owns that), nor does it leak any + * storage mechanism (no file handles, NDJSON, index, or GC in the contract). + * Split out from the POC's combined store so transcript retention can evolve + * without touching channel metadata. + */ +export interface ITranscriptStore { + /** Appends one stamped `TurnEvent` to a turn's log. */ + appendTurnEvent(args: AppendTurnEventArgs): Promise + + /** Reads all persisted events for a turn, ordered by `seq` ascending. */ + readTurnEvents(args: ReadTurnEventsArgs): Promise +} diff --git a/src/server/core/interfaces/executor/i-curate-executor.ts b/src/server/core/interfaces/executor/i-curate-executor.ts index 889b53072..32a5ece2e 100644 --- a/src/server/core/interfaces/executor/i-curate-executor.ts +++ b/src/server/core/interfaces/executor/i-curate-executor.ts @@ -1,4 +1,6 @@ import type {ICipherAgent} from '../../../../agent/core/interfaces/i-cipher-agent.js' +// TODO(ENG-3034): relocate the HtmlWriteError type into core so this stops importing infra. +// eslint-disable-next-line no-restricted-imports import type {HtmlWriteError} from '../../../infra/render/writer/html-writer.js' import type {CurateUsageRecord} from '../../domain/entities/curate-log-entry.js' import type {IUsageAggregator} from '../telemetry/i-usage-aggregator.js' diff --git a/src/shared/types/channel.ts b/src/shared/types/channel.ts index 0f4a36997..25c26d08c 100644 --- a/src/shared/types/channel.ts +++ b/src/shared/types/channel.ts @@ -69,6 +69,7 @@ export const ContentBlockSchema = z.discriminatedUnion('type', [ ResourceLinkContentBlockSchema, EmbeddedResourceContentBlockSchema, ]) + export type ContentBlock = z.infer // ─── Handle ───────────────────────────────────────────────────────────────── @@ -93,6 +94,7 @@ export const TurnAuthorSchema = z.discriminatedUnion('kind', [ sessionId: z.string().optional(), }), ]) + export type TurnAuthor = z.infer // ─── ChannelMember ────────────────────────────────────────────────────────── @@ -137,6 +139,7 @@ export const ChannelMemberAcpAgentSchema = z.object({ capabilities: z.array(z.string()), status: AcpAgentStatusSchema, }) + export type ChannelMemberAcpAgent = z.infer export const ChannelMemberLocalAgentSchema = z.object({ @@ -146,12 +149,14 @@ export const ChannelMemberLocalAgentSchema = z.object({ agentName: z.string(), status: LocalAgentStatusSchema, }) + export type ChannelMemberLocalAgent = z.infer export const ChannelMemberSchema = z.discriminatedUnion('memberKind', [ ChannelMemberAcpAgentSchema, ChannelMemberLocalAgentSchema, ]) + export type ChannelMember = z.infer /** @@ -165,11 +170,13 @@ export const ChannelMemberSummarySchema = z.object({ status: z.string().optional(), capabilities: z.array(z.string()).optional(), }) + export type ChannelMemberSummary = z.infer // ─── Turn + TurnDelivery ──────────────────────────────────────────────────── export const TurnStateSchema = z.enum(['pending', 'dispatched', 'completed', 'cancelled']) + export type TurnState = z.infer export const TurnSchema = z @@ -186,6 +193,7 @@ export const TurnSchema = z idempotencyKey: z.string().optional(), }) .strict() + export type Turn = z.infer export const TurnDeliveryStateSchema = z.enum([ @@ -197,6 +205,7 @@ export const TurnDeliveryStateSchema = z.enum([ 'cancelled', 'errored', ]) + export type TurnDeliveryState = z.infer export const TurnDeliverySchema = z @@ -223,6 +232,7 @@ export const TurnDeliverySchema = z finalAnswer: z.string().optional(), }) .strict() + export type TurnDelivery = z.infer // ─── TurnEvent ────────────────────────────────────────────────────────────── @@ -242,6 +252,7 @@ export const PermissionOptionSchema = z.object({ name: z.string(), kind: z.enum(['allow_once', 'allow_always', 'reject_once', 'reject_always']), }) + export type PermissionOption = z.infer export const TurnEventSchema = z.discriminatedUnion('kind', [ @@ -308,6 +319,7 @@ export const TurnEventSchema = z.discriminatedUnion('kind', [ }) .strict(), ]) + export type TurnEvent = z.infer // ─── Channel ──────────────────────────────────────────────────────────────── @@ -316,6 +328,7 @@ export const ChannelSettingsSchema = z.object({ maxParallelAgents: z.number().int().positive().optional(), defaultLookbackTurns: z.number().int().nonnegative().optional(), }) + export type ChannelSettings = z.infer export const ChannelSchema = z @@ -330,4 +343,5 @@ export const ChannelSchema = z settings: ChannelSettingsSchema.optional(), }) .strict() + export type Channel = z.infer