fix review findings: stale abort() docs + move RFC to implemented

Codex's no-ship was a completeness/docs-sync gap, not loop behavior:

- docs/architecture.md: drop the public abort() handle row; the teardown
  signal is now cancel() then await whenIdle().
- cancel.spec.ts: the module doc and the turn-start comment contrasted
  cancel() against a public abort() verb that no longer exists — reword to
  name the loop's private step AbortController.
- packages/ui/acp/src/index.ts: the post-resume-leak comment cited abort();
  cancel() is the surviving stop verb that likewise does not unregister.
- Move the RFC proposed -> implemented/simplification with amended text:
  Status flips, the both-removal proposal is narrowed to abort-only, and an
  implementation note records why whenIdle() is retained (load-bearing
  quiescence primitive with live ACP consumers). Update docs/rfc/README.md.
- AGENTS.md "RFCs are proposals, not golden truth": add the concrete
  abort/whenIdle worked example now that the implemented RFC exists to link.
- Regenerate the cordis catalog (line-number drift from the rebase).
This commit is contained in:
Tianyi Cui
2026-06-21 09:10:56 +08:00
parent f6bd1468f2
commit c6ed980d6f
8 changed files with 62 additions and 54 deletions

View File

@@ -112,9 +112,8 @@ Tool schemas are deliberately **part of the assembly**: "what the model is told
- `send(content)` — queued message; starts a turn when idle, else next turn
- `steer(content)` — mid-turn injection, drained **between steps**; behaves like `send` when idle
- `inject(content)` — in-session context (`context/message` event); the next request sees it (Claude Code attachment / system-reminder analog). An inject made while the agent is *running* joins the open turn; an inject while *idle* is wrapped in a one-shot turn (`turn/start{trigger:injection}``context/message``turn/end`) so every event stays turn-enclosed (see [the turn-enclosure invariant](rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md)).
- `abort(reason)` — aborts the in-flight step via `AbortSignal`
- `cancel(reason)` — the single public stop primitive: clears queued + steering work, aborts the in-flight step, and drops a turn about to start (the pre-step window) so a queued-but-not-started prompt never runs and cannot be batched into the cancelled turn. A UI/ACP `session/cancel` maps to it.
- `whenIdle()` — resolves once the agent reaches quiescence after settling out of `running` (resolves immediately when already idle; awaits the loop exit when disposed). The teardown signal: `abort()` then `await whenIdle()` guarantees the in-flight turn has fully stopped. Observes the transition without disposing the agent.
- `whenIdle()` — resolves once the agent reaches quiescence after settling out of `running` (resolves immediately when already idle; awaits the loop exit when disposed). The teardown signal: `cancel()` then `await whenIdle()` guarantees the in-flight turn has fully stopped. Observes the transition without disposing the agent.
- `session`, `status`, `options`
**TODO(sub-agents)**: `spawn`/`fork` land on `AgentLoop.create()` — fork seeds the child Session with the parent's event log, spawn starts fresh; children are ordinary `Agent` handles so `steer()` and event subscription work uniformly. Inter-agent channels beyond these primitives are deliberately deferred.

View File

@@ -25,7 +25,7 @@ An agent was registered in the AgentRegistry and is ready to receive messages.
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:136`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:137`](../../packages/core/agent/src/types.ts)
#### `agent/disposed` — emit
@@ -37,7 +37,7 @@ An agent was disposed and removed from the registry; its fiber and any in-flight
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:142`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:143`](../../packages/core/agent/src/types.ts)
#### `agent/error` — emit
@@ -49,7 +49,7 @@ A step or turn errored. The loop reports a failure here (plus the logger) even w
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:219`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:220`](../../packages/core/agent/src/types.ts)
#### `agent/queued` — emit
@@ -61,7 +61,7 @@ A message entered the agent's inbox (queued or steering). `source` is the resolv
Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:155`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:156`](../../packages/core/agent/src/types.ts)
#### `agent/request` — waterfall
@@ -73,7 +73,7 @@ Waterfall: mutate the fully-assembled GenerateOptions before the model call (hoo
Types: [Agent](../core-data-structures/core.md) · [GenerateOptions](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:188`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:189`](../../packages/core/agent/src/types.ts)
#### `agent/status` — emit
@@ -85,7 +85,7 @@ Agent status changed (`idle` ⇄ `running`, or → `disposed`). Drive lifecycle
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:149`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:150`](../../packages/core/agent/src/types.ts)
#### `agent/steering` — emit
@@ -97,7 +97,7 @@ Steering content was injected into a running turn.
Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:213`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:214`](../../packages/core/agent/src/types.ts)
#### `agent/step-end` — emit
@@ -109,7 +109,7 @@ A step ended.
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:179`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:180`](../../packages/core/agent/src/types.ts)
#### `agent/step-result` — waterfall
@@ -121,7 +121,7 @@ Waterfall: post-process the assembled assistant Message before tool dispatch (va
Types: [Agent](../core-data-structures/core.md) · [Message](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:194`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:195`](../../packages/core/agent/src/types.ts)
#### `agent/step-start` — emit
@@ -133,7 +133,7 @@ A step (one model call plus its tool dispatch) began. `step` is 1-based within t
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:174`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:175`](../../packages/core/agent/src/types.ts)
#### `agent/stream-chunk` — emit
@@ -145,7 +145,7 @@ A raw StreamChunk arrived from the model (token-level UI/log feed).
Types: [Agent](../core-data-structures/core.md) · [StreamChunk](../core-data-structures/llm-streaming.md)
Source: [`packages/core/agent/src/types.ts:208`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:209`](../../packages/core/agent/src/types.ts)
#### `agent/turn-continuation` — waterfall
@@ -157,7 +157,7 @@ Waterfall: override the turn-continuation decision. The default (computed by the
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:201`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:202`](../../packages/core/agent/src/types.ts)
#### `agent/turn-end` — emit
@@ -169,7 +169,7 @@ A turn ended. `reason` distinguishes a clean stop from a truncated or aborted on
Types: [Agent](../core-data-structures/core.md) · [TurnEndReason](../core-data-structures/session.md)
Source: [`packages/core/agent/src/types.ts:168`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:169`](../../packages/core/agent/src/types.ts)
#### `agent/turn-start` — emit
@@ -181,7 +181,7 @@ A turn began. `turn` is the 1-based turn number within the session.
Types: [Agent](../core-data-structures/core.md)
Source: [`packages/core/agent/src/types.ts:162`](../../packages/core/agent/src/types.ts)
Source: [`packages/core/agent/src/types.ts:163`](../../packages/core/agent/src/types.ts)
### `llm/*`
@@ -288,12 +288,12 @@ The agent-loop plugin (`ctx.agentLoop`): creates ReactLoopAgents, runs their loo
The loop itself is deliberately thin — every behavior beyond "call the model, run the tools, repeat" belongs to plugins listening on the event taxonomy declared in @deepseek-ai/dsh-agent.
```ts cordis-catalog
create(id: string, options: AgentOptions = {}): ReactLoopAgent
create(id: AgentId, options: AgentOptions = {}): ReactLoopAgent
createAgent(options: CreateAgentOptions): AgentHandle
async resume(options: ResumeAgentOptions): Promise<AgentHandle>
```
Source: [`packages/core/agent-loop/src/index.ts:60`](../../packages/core/agent-loop/src/index.ts)
Source: [`packages/core/agent-loop/src/index.ts:63`](../../packages/core/agent-loop/src/index.ts)
### `ctx.agents` — `AgentRegistry`
@@ -327,7 +327,9 @@ Semantics every implementation must honor:
abstract resolve(request: BashExecRequest): BashExecSpec
abstract run(spec: BashExecSpec): Promise<BashRunResult>
abstract start(spec: BashExecSpec): BashTask
abstract get(id: BashTaskId): BashTask | undefined
abstract ownerOf(id: BashTaskId): OwnerToken | undefined
abstract list(): BashTask[]
abstract readOutput(id: BashTaskId): BashTaskRead
abstract kill(id: BashTaskId): boolean
onTaskDone(listener: BashTaskListener): () => void

View File

@@ -51,7 +51,6 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r
|---|---|
| [Unify the agent id and the session id](proposed/simplification/2026-06-20-unify-agent-and-session-id.md) | 2026-06-20 |
| [Stop mirroring durable boundaries as agent events](proposed/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) | 2026-06-20 |
| [Keep one public stop primitive](proposed/simplification/2026-06-20-public-agent-stop-surface.md) | 2026-06-20 |
| [Fold trace-only session facts into load-bearing events](proposed/simplification/2026-06-20-collapse-trace-only-session-events.md) | 2026-06-20 |
### Architecture
@@ -95,6 +94,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r
| [Drop unconsumed assembled LLM convenience surfaces](implemented/simplification/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md) | 2026-06-20 |
| [Drop the unconsumed `llm/adapter-change` event](implemented/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) | 2026-06-20 |
| [Prune dead methods from the persistence and bash seams](implemented/simplification/2026-06-20-prune-dead-seam-methods.md) | 2026-06-20 |
| [Keep one public stop primitive](implemented/simplification/2026-06-20-public-agent-stop-surface.md) | 2026-06-20 |
### Architecture

View File

@@ -0,0 +1,36 @@
# RFC: Keep one public stop primitive
Status: implemented (proposed 2026-06-20; accepted in amended form — `whenIdle()` retained)
> **Implementation note (scope narrowed from the original proposal).** This RFC proposed removing BOTH `abort()` and `whenIdle()` from the public `Agent` handle. Only `abort()` was removed. Validating the premise against the code ([AGENTS.md "RFCs are proposals, not golden truth"](../../../../AGENTS.md)) found `whenIdle()` to be a **load-bearing quiescence primitive**, not dead surface: it is the settle signal in several ACP tests (`packages/ui/acp/tests/{edges,turns,dispose}.spec.ts`) and is backed by a deliberate loop contract (settle waiters without a status transition; handle the replacement-turn race). The RFC's suggested migration — have consumers observe the `running`→`idle` transition by hand — is exactly the brittle hand-rolled path [AGENTS.md § Defensive patterns](../../../../AGENTS.md) warns against ("Async state is not synchronous state"). Deleting a clean primitive to push every consumer onto that is a net loss, so `whenIdle()` stays. `abort()` was genuinely dead public surface (no production caller; the loop aborts its own `AbortController` directly), so it was removed as proposed. The text below is amended to describe what shipped.
## Problem
The public `Agent` handle exposed two overlapping ways to stop in-flight work: `abort(reason?)` and `cancel(reason?)`. `abort()` killed only the in-flight step and left queued work alone; `cancel()` clears queued and steering work, aborts the running step, and handles the pre-step race. In production, ACP uses `cancel()` for `session/cancel`, while lifecycle owners tear down agents through `AgentHandle.dispose()`. No production caller needed bare `abort()`.
The `abort()`/`cancel()` distinction is real — `abort()` preserves queued prompts and steering while `cancel()` drops them — but no shipping code called the public `abort()` verb. The loop's own stop paths (`cancel()` and disposal) abort the current `AbortController` directly rather than routing through `Agent.abort()`. Most tests that called `abort()` interrupt an empty queue and switch to `cancel(reason)`; the steering re-delivery test that deliberately depends on queue preservation drives the in-flight `AbortController` directly, because `cancel()` would drop the queued steering it is trying to prove survives a step abort. The no-argument `abort()` default reason (`'aborted'`) is deleted with the verb rather than preserved by accident; `cancel()` keeps its own `'cancelled'` default.
The extra surface area made the loop carry a public verb that is mostly a teardown internal: `abort()` had to be documented as distinct from queue-aware cancellation even though a UI cancellation almost always wants the broader operation.
## Proposal
Keep `cancel()` as the only public *stop* primitive on `Agent`. Lifecycle owners use `AgentHandle.dispose()` to stop and unregister an agent; non-owners use `cancel()` to abandon current and queued work. The implementation keeps a private abort controller, but it is not part of the plugin-facing `Agent` contract.
`whenIdle()` is **retained** as the public quiescence-observation primitive (resolve once the agent settles out of `running`, resolve immediately when already idle, await the loop exit when disposed). It is not a stop verb; it is how a non-owner observes the stop *completing* without disposing the agent, and it has live consumers (the ACP bridge's settle points).
Delete public `abort()`, the tests that exercise it as standalone API, and the docs that describe step-only abort as an embedding feature. Empty-queue abort tests migrate to `cancel(reason)` where they still prove cancellation behavior; tests whose subject is the loop's internal `AbortController` behavior drive that controller directly via an in-package typed cast to the private field; tests that only pin the removed no-arg `abort()` default go away with the method. The disposer remains async and still waits for the loop to stop.
## Acceptance criteria
- `Agent` exposes no public `abort()`; `cancel()`, `whenIdle()`, and `steer()` remain part of the surface.
- ACP cancellation continues to call `cancel()`.
- Agent teardown continues to await quiescence through handle disposal, and `whenIdle()` still resolves on quiescence for non-owner observers.
- Tests cover cancellation and disposal as the two supported stop paths.
## What we give up
A future plugin cannot abort only the current model/tool step while preserving queued prompts through the public interface. If that use case becomes real, it should return with a named consumer and a narrower contract. Today it is latent generality that keeps a private loop mechanic public.
## Related
This RFC only removes the redundant stop verb. Mid-turn steering remains an intentional message path; quiescence observation remains via `whenIdle()`. The resulting public surface is `send()`, `steer()`, `inject()`, `cancel()`, `whenIdle()`, status, options, session, and identity.

View File

@@ -1,32 +0,0 @@
# RFC: Keep one public stop primitive
Status: proposed
## Problem
The public `Agent` handle exposes three ways to reason about stopping work: `abort(reason?)`, `cancel(reason?)`, and `whenIdle()`. `abort()` kills only the in-flight step and leaves queued work alone; `cancel()` clears queued and steering work, aborts the running step, and handles the pre-step race; `whenIdle()` exposes the loop's private quiescence waiter to any consumer. In production, ACP uses `cancel()` for `session/cancel`, while lifecycle owners tear down agents through `AgentHandle.dispose()`. No production caller needs bare `abort()` or `whenIdle()`.
The `abort()`/`cancel()` distinction is real — `abort()` preserves queued prompts and steering while `cancel()` drops them — but no shipping code calls the public `abort()` verb. The loop's own stop paths (`cancel()` and disposal) abort the current `AbortController` directly rather than routing through `Agent.abort()`. Most tests that call `abort()` interrupt an empty queue and can switch to `cancel(reason)`; the one steering re-delivery test that deliberately depends on queue preservation should drive the in-flight `AbortController` directly, because `cancel()` would drop the queued steering it is trying to prove survives a step abort. The no-argument `abort()` default reason (`'aborted'`) is also deleted with the verb rather than preserved by accident; `cancel()` keeps its own `'cancelled'` default.
The extra surface area makes the loop carry public semantics that are mostly teardown internals. `whenIdle()` needs waiter state, special disposed-agent behavior, and a loop-exit promise so it resolves after quiescence rather than merely after a status flip. `abort()` has to be documented as distinct from queue-aware cancellation even though a UI cancellation almost always wants the broader operation.
## Proposal
Keep `cancel()` as the only public stop primitive on `Agent`. Lifecycle owners use `AgentHandle.dispose()` to stop and unregister an agent; non-owners use `cancel()` to abandon current and queued work. The implementation can keep private abort controllers and quiescence promises, but they are not part of the plugin-facing `Agent` contract.
Delete public `abort()` and `whenIdle()`, the tests that exercise them as standalone API, and the docs that describe step-only abort as an embedding feature. Empty-queue abort tests migrate to `cancel(reason)` where they still prove cancellation behavior; tests whose subject is the loop's internal `AbortController` behavior drive that controller directly; tests that only pin the removed no-arg `abort()` default go away with the method. The disposer remains async and still waits for the loop to stop; that guarantee moves entirely onto `AgentHandle.dispose()`.
## Acceptance criteria
- `Agent` exposes no public `abort()` or `whenIdle()`; `steer()` remains part of the message surface.
- ACP cancellation continues to call `cancel()`.
- Agent teardown continues to await quiescence through handle disposal.
- Tests cover cancellation and disposal as the two supported stop paths.
## What we give up
A future plugin cannot abort only the current model/tool step while preserving queued prompts through the public interface. If that use case becomes real, it should return with a named consumer and a narrower contract. Today it is latent generality that keeps private loop mechanics public.
## Related
This RFC only removes the stop/quiescence methods. Mid-turn steering remains an intentional message path; the resulting public surface is `send()`, `steer()`, `inject()`, `cancel()`, status, options, session, and identity.