Merge branch 'codex/simp-agent-entry-state' into codex/simp-unify-agent-session-id
# Conflicts: # docs/config-catalog.md # docs/cordis-catalog/services.md # docs/event-producer-consumer.md # docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md # docs/rfc/implemented/architecture/2026-06-20-branded-ids.md # docs/rfc/implemented/simplification/2026-06-20-remove-agent-boundary-mirror-events.md # docs/rfc/proposed/feature/2026-07-07-claude-code-and-codex-subagent-backends.md # docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md # packages/bash/tool-bash/README.md # packages/bash/tool-bash/src/index.ts # packages/bash/tool-bash/tests/tools.spec.ts # packages/core/agent-loop/README.md # packages/core/agent-loop/tests/properties.spec.ts # packages/core/agent/README.md # packages/core/agent/src/index.ts # packages/guard/repeat-tool-guard/src/index.ts # packages/hooks/hooks-claude/tests/bridge.spec.ts # packages/subagent/subagent-acp/tests/mock-acp-server.ts # packages/ui/acp/tests/dispose.spec.ts # packages/ui/stdio-agent/src/index.ts # packages/ui/stdio-agent/src/stdio-chat.ts # packages/ui/stdio-agent/tests/stdio-chat.spec.ts # packages/util/brand/src/index.ts
This commit is contained in:
Tianyi Cui
@@ -6,14 +6,14 @@ Status: proposed
|
||||
|
||||
The harness models its core vocabulary — content blocks, message sources, finish reasons, turn triggers, turn-end reasons, and session events — as **merge-extensible maps**: a TypeScript `interface` (e.g. `SessionEventMap`, `ContentBlockMap`) that plugins augment via declaration merging, with the public union derived as `Map[keyof Map]`. This is the repo's universal extension pattern, documented in [docs/architecture.md](../../../architecture.md) ("The same merge-extensible-map pattern is used for `MessageSource`, `FinishReason`, `TurnTrigger`, and `TurnEndReason`") and relied on by the `defineTool` `InferArgs` DSL and the `assertNever` exhaustiveness convention.
|
||||
|
||||
The pattern is **compile-time only**. The types vanish at runtime: there is no schema object to validate an incoming value against, parse untrusted input with, or enumerate at runtime. Two concrete consequences surfaced in review of [the session-persistence work](../../implemented/architecture/2026-06-14-session-persistence.md) (#33):
|
||||
The pattern is **compile-time only**. The types vanish at runtime: there is no schema object to validate an incoming value against, parse untrusted input with, or enumerate at runtime. The [session-persistence contract](../../implemented/architecture/2026-06-14-session-persistence.md) exposes two consequences:
|
||||
|
||||
1. **Persistence treats `event.data` as opaque JSON.** The JSONL/SQLite backends `JSON.stringify`/`JSON.parse` each event verbatim; the only runtime guard is `isJsonValue` (round-trip serializability — rejects BigInt, functions, cycles, non-finite numbers, …), NOT structural validation. A corrupted-but-still-JSON event datum (wrong field types, missing fields) round-trips silently and is only caught later, if at all, by a consumer's `switch`.
|
||||
2. **No runtime contract for plugin-added variants.** A plugin that declaration-merges a new `SessionEventMap` key gets compile-time typing for its own code, but nothing validates that the values it produces match the shape it declared — at the producer, at the persistence boundary, or on reload.
|
||||
|
||||
A reviewer asked whether the project should move "all the JSON serialization/deserialization" — and ultimately the event vocabulary itself — to **Zod** (or a similar runtime-schema library), so the durable boundary and the plugin extension points are backed by runtime schemas rather than erased types.
|
||||
This raises whether the event vocabulary should move to **Zod** or another runtime-schema library so durable and plugin boundaries have runtime schemas rather than erased types.
|
||||
|
||||
This RFC scopes that question. It does **not** propose an implementation; it records the tradeoff so the decision is made deliberately rather than incrementally inside a persistence PR.
|
||||
This RFC scopes that question without proposing an implementation.
|
||||
|
||||
## Why this is not a persistence change
|
||||
|
||||
@@ -32,7 +32,7 @@ A migration of the event/vocabulary surface to runtime schemas touches, at minim
|
||||
- **The `defineTool` `InferArgs` DSL** (`dsh-tools`), which derives zero-cast `execute` arg types from a compile-time schema spec — the showcase of the current approach.
|
||||
- **Docs**: architecture.md (the pattern is described as foundational), [dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md), and any RFC that references the pattern.
|
||||
|
||||
This is a HUGE change. It is not in scope for the RFC-009 session-persistence work and must not be smuggled in through it.
|
||||
This is a repository-wide vocabulary redesign, not a persistence implementation detail.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
@@ -46,7 +46,7 @@ Keep the compile-time pattern. Persistence stays opaque-JSON + serializability g
|
||||
Tighten only the genuinely-closed shapes that already have hand-rolled type guards — e.g. the JSONL `HeaderLine` guard (`isHeaderLine`) — using **schemastery** (the repo's existing schema library, already used for every plugin `static Config`). Leave the merge-extensible event union as-is.
|
||||
|
||||
- **Pros**: small, fits the existing convention (schemastery, not a new lib); replaces hand-rolled guards on closed shapes with declarative schemas; no core redesign.
|
||||
- **Cons**: does not address event-data validation (the thing the reviewer actually asked about); only helps the fixed metadata records.
|
||||
- **Cons**: does not address event-data validation; only the fixed metadata records improve.
|
||||
|
||||
### C. Runtime schema registry for the whole vocabulary (Zod or schemastery)
|
||||
Replace the merge-extensible maps with a runtime registry the producers contribute to and the persistence/consumer paths validate against.
|
||||
@@ -56,11 +56,11 @@ Replace the merge-extensible maps with a runtime registry the producers contribu
|
||||
|
||||
## Proposal
|
||||
|
||||
Defer. Do **not** change #33. If runtime validation is wanted at the durable boundary in the near term, **Option B** (schemastery on the closed header/metadata shapes) is the proportionate step and stays within the existing convention. **Option C** is a genuine architecture decision that should be evaluated on its own merits — including whether the chosen library is Zod or schemastery — and, if accepted, land as its own change with its own RFC, not as a side effect of persistence serialization.
|
||||
Defer. If runtime validation is wanted at the durable boundary, **Option B** (schemastery on closed header and metadata shapes) is the proportionate step within the existing convention. **Option C** is an architecture decision that requires its own implementation RFC, including a choice between Zod and schemastery.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The decision state is explicit: Option C proceeds only as its own change with its own implementation RFC — never as a side effect of a persistence PR.
|
||||
- Option C proceeds only through its own implementation RFC, never as a persistence side effect.
|
||||
- If Option B is taken up, the closed header/metadata shapes (the JSONL `isHeaderLine` guard and kin) validate through schemastery in place of hand-rolled guards, with the merge-extensible maps untouched.
|
||||
|
||||
## Risks
|
||||
|
||||
@@ -24,7 +24,7 @@ The runtime should own:
|
||||
|
||||
## Current seam consumption
|
||||
|
||||
A consumer census of the surface the runtime would carve up. Production has two seam consumers: `packages/bash/tool-bash/src/index.ts` consumes `resolve`, `run`, `start`, `ownerOf`, `readOutput`, `kill`, and `onTaskDone`; and the hook bridges — via `dsh-hook-protocol`'s `runHook` (`packages/hooks/hook-protocol/src/runner.ts`) — consume `resolve` + `run` only, a foreground-only trusted-plugin caller that sets the seam's `stdin`/`env` fields, so the background machinery stays single-consumer (which sharpens the extraction premise). `get()`/`list()` have test-harness consumers only — they were removed once and reverted on the merits (the implementation note in [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records the test-migration cost dwarfing the surface removed). The per-task `BashTask.done` promise has no consumer through the public seam either (`dsh-tool-bash` completes via `onTaskDone`), but it is production-load-bearing INSIDE the implementation: `dsh-bash-local`'s disposal awaits it to reach quiescence. The seam therefore exposes two public completion representations — the per-task promise and the global `onTaskDone` listener registry — and the shipped consumers use only the latter: the runtime should pick exactly one public completion surface and record which. Two shape facts for the split to dissolve or preserve deliberately: `BashExecSpec.timeoutMs` is required but ignored by `start()` (documented in the seam JSDoc itself), and `stdin`/`env` ride the shared spec for the foreground trusted-plugin path — the carve-up must keep a plain in-process foreground `resolve`+`run` path carrying them, so hook execution is never forced through the long-running runtime. Adjacent blast radius: the credential scrub is duplicated between the two production spawn sites (`packages/bash/bash-local/src/run.ts` and `packages/subagent/subagent-acp/src/run.ts`); if the runtime absorbs spawn-env policy, collapsing that duplication is its work too.
|
||||
Current consumers split cleanly: `dsh-tool-bash` uses the full foreground/background seam, while hook bridges use only foreground `resolve` and `run` with trusted `stdin` and `env`. `get` and `list` are test-only; `BashTask.done` is implementation-only for disposal, while production completion uses `onTaskDone`. An extracted runtime should expose one public completion mechanism, preserve the simple foreground path for hooks, and decide whether background `timeoutMs` belongs on `start`. If it owns process spawning, it should also centralize the duplicated credential scrub.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
|
||||
@@ -4,45 +4,36 @@ Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
A user deep in a live session often wants to fork the conversation — ask "why did we structure it this way", explore an alternative, get an explanation — without polluting the main context and without abandoning the surface they are in. The harness has every primitive this needs and no product face for it: [the session-store fork API](../../implemented/feature/2026-06-30-session-store-fork-api.md) produces a `Session` with no attached agent and no way for a client to reach it, and [the fork subagent](../../implemented/feature/2026-06-21-subagent-capability-seam.md) seeds a child with the parent's prefix but runs it as a model-driven task whose whole transcript collapses into one tool result — neither yields a forked conversation the USER can talk to.
|
||||
|
||||
The return path is missing entirely: nothing carries a conclusion from one branch back into another. Whatever the user learns in an exploration branch is copy-pasted by hand or lost, with no provenance and no replayable record.
|
||||
|
||||
Terminal-first competitors ship half of each: single-shot side questions with inherited context, and user-switched branch copies that force a client restart. None ship the return verb. A harness that owns its session store and context assembly can do both cheaply — and can do so without breaking the provider prefix cache, which third-party wrappers structurally cannot.
|
||||
A user may want to explore a question from a live session without changing its main context. Existing primitives do not expose that product shape: [session-store fork](../../implemented/feature/2026-06-30-session-store-fork-api.md) creates an unattached session, while [fork subagents](../../implemented/feature/2026-06-21-subagent-capability-seam.md) are model-driven tasks whose transcript collapses into one tool result. Neither gives the user a separate conversation, and neither records a conclusion back into the parent with provenance.
|
||||
|
||||
## Proposal
|
||||
|
||||
A **side session** is an ordinary live session forked from a source session at its last completed turn, attached to its own agent, framed as a read-only advisor, with one new verb — **merge-back** — that hands a condensed note to the parent.
|
||||
A **side session** is an ordinary live session forked at the source's last completed turn, attached to its own agent, framed as a read-only advisor, and able to **merge back** one condensed note.
|
||||
|
||||
- **Fork + attach composes existing primitives.** The child is created via `ctx.agents.create({ seed, meta })` with the parent's balanced completed-turn prefix (the same slice the fork subagent takes) and `parentSession`/`seedLength` lineage stamped in `meta`. No new core service and no session-store change, for the same reasons [the fork API RFC](../../implemented/feature/2026-06-30-session-store-fork-api.md) rejected a standalone fork service.
|
||||
- **Advisor framing rides the log, not the system prompt.** The rules ("you are a read-only side advisor; explain, do not mutate; refuse task continuation") are `inject()`ed as a `context/message` with source `{ kind: 'plugin', plugin: 'sidechat' }` immediately after creation. The child's system prompt stays byte-identical to the parent's, so the provider's prefix cache covers the inherited history.
|
||||
- **Merge-back is one condense turn plus one injection.** The child is prompted for a bounded handback note (a hard length cap), which is then `inject()`ed into the PARENT as a `context/message` with the same plugin source. The parent's next request sees it at its chronological position; replay and [reconstructability](../../implemented/architecture/2026-07-05-reconstructable-requests.md) hold by construction; no new event type enters [the session vocabulary](../../../core-data-structures/session.md).
|
||||
- **The surface binding is deliberately unspecified.** How a user invokes the fork and the merge, and how a handback note is presented, are client concerns; while the harness speaks through protocols whose UI it does not control, this RFC pins only the surface-agnostic mechanics above and leaves presentation to the first surface the project owns.
|
||||
- **Fork and attach:** create the child with the parent's balanced completed-turn prefix and stamp `parentSession` and `seedLength` in its metadata. This composes `ctx.agents.create({ seed, meta })`; it adds no core service or session-store method.
|
||||
- **Advisor framing:** inject one plugin-sourced `context/message` after creation that tells the child to explain without mutating or continuing the task. Keeping the system prompt byte-identical preserves the provider prefix cache over inherited history.
|
||||
- **Merge-back:** ask the child for a length-capped handback, then inject one plugin-sourced `context/message` into the parent. The next parent request sees it at its logged position, preserving replay and [request reconstructability](../../implemented/architecture/2026-07-05-reconstructable-requests.md) without a new session event.
|
||||
- **Presentation:** invocation, session switching, and handback rendering belong to the first client-owned surface. This RFC specifies only the surface-independent mechanics.
|
||||
|
||||
Scope also excludes: rewind productization (mechanically the same fork-to-an-earlier-boundary, an entirely different product question), session tree views, a model-facing side-session tool, and `forkName`/`mergedInto` metadata.
|
||||
|
||||
## Prototype
|
||||
|
||||
A spike (branch `spike/side-sessions-b`) validates the mechanics against the live adapter: a fork that leaves the source log untouched, a child seeded with the full inherited prefix, a multi-turn advisor exchange that demonstrably uses parent history, and a merge-back the parent's next turn quotes correctly.
|
||||
Rewind productization, session-tree views, a model-facing side-session tool, and `forkName`/`mergedInto` metadata are out of scope. A live-adapter spike validated source-log isolation, inherited context, a multi-turn child exchange, and merge-back visibility in the parent's next turn.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Carry side conversations through the subagent seam.** Rejected: a subagent child is a model-driven run — the parent's model spawns it, drives it, and consumes its result as a tool result. A side session is user-driven, needs its own client-visible session and lifecycle, and must outlive any single parent turn.
|
||||
- **Persona via `system-prompt/assemble` section filtering.** Rejected as the default path: any system-prompt byte change invalidates the provider prefix cache from token 0, forfeiting the cheap fork that makes side sessions attractive on long histories. The filter seam remains available for deployments that prefer hard prompt separation over cache reuse.
|
||||
- **A dedicated `sidechat/*` event family for the handback.** Deferred: `context/message` with a mandatory plugin source already satisfies durability, provenance, and replay. A first-class event earns its catalog, persistence, and snapshot costs only if a UI needs to render handbacks as dedicated cards.
|
||||
- **Binding the proposal to a protocol surface now.** Rejected in review: the harness currently speaks through client-owned UIs it does not control, so any presentation contract written today would be speculative. The RFC pins the surface-agnostic mechanics and defers presentation to the first surface the project owns.
|
||||
- **Surface-level mirroring of the handback.** Rejected in review: a visible record emitted outside the log vanishes on replay while the model still sees it. Whatever surface eventually renders the handback must derive its presentation from the durable `context/message`, so live and replayed views come from the same event.
|
||||
- **Use the subagent seam:** rejected because side sessions are user-driven, client-visible, and may outlive a parent turn; subagents are model-driven runs returning one tool result.
|
||||
- **Change the child system prompt:** rejected by default because any byte change invalidates the prefix cache from token zero. Deployments may still prefer that stronger separation.
|
||||
- **Add `sidechat/*` events:** deferred because a sourced `context/message` already provides durability, provenance, and replay. A dedicated event is justified only by a surface that needs distinct rendering.
|
||||
- **Bind a protocol surface now:** rejected because current UIs are client-owned. Live presentation must eventually derive from the durable message so replay renders the same record.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- Forking a live session yields a child agent seeded with the source's balanced completed-turn prefix, with `parentSession` and `seedLength` in its header and a system prompt byte-identical to the parent's; the source log is untouched by the fork.
|
||||
- The advisor framing is exactly one plugin-sourced `context/message` at the head of the child's appended history — never a system-prompt change.
|
||||
- Merge-back appends exactly one length-capped `context/message` to the parent with source `plugin: sidechat`; the parent's next request sees it, and replay reproduces it at the same position.
|
||||
- Parent and child run concurrently without cross-talk between their logs or streams.
|
||||
- Coverage: unit tests for the fork/attach and merge-back mechanics; surface-level snapshot coverage lands with whichever surface first binds the feature.
|
||||
- Forking leaves the source untouched and creates a child with the balanced completed-turn prefix, `parentSession`, `seedLength`, and a byte-identical system prompt.
|
||||
- Advisor framing adds exactly one plugin-sourced `context/message` at the head of the child's appended history, rather than changing its system prompt.
|
||||
- Merge-back adds exactly one length-capped `context/message` with source `plugin: sidechat`; the next parent request and replay see it at the same position.
|
||||
- Parent and child run concurrently without log or stream cross-talk.
|
||||
- Unit tests cover fork/attach and merge-back; snapshot coverage lands with the first bound surface.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Read-only is advisory in v1.** The rules are injected context, not enforcement; a determined prompt can still drive mutating tools. The hard gate is a `tools/pre-execute` deny via [the interception seams](../../implemented/feature/2026-06-30-interception-seams.md), and the RFC's advisor framing is written so that gate can be added without changing the mechanics.
|
||||
- **A compacted source forks its compacted view.** The child inherits the summary, not the original turns; whichever surface binds the feature should disclose this once [compaction](../../implemented/feature/2026-06-18-compaction-capability-seam.md) ships in this path.
|
||||
- **Handback notes spend parent tokens.** The length cap and one-note-per-merge bound the cost, but a user who merges repeatedly accumulates notes; a future consolidation pass belongs to the compaction work, not here.
|
||||
- Read-only behavior is advisory until a `tools/pre-execute` deny gate enforces it; [the interception seam](../../implemented/feature/2026-06-30-interception-seams.md) can add that gate without changing these mechanics.
|
||||
- A compacted source forks its compacted view, so a bound surface should disclose that the child inherits summaries rather than replaced turns.
|
||||
- Repeated handbacks consume parent context. The per-merge length cap bounds each note; later consolidation belongs to compaction.
|
||||
|
||||
@@ -4,7 +4,7 @@ Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` and the root `tsconfig.json` each hand-list every package as explicit project `references` — two identical sets that grow in lockstep, so a single generator can emit both — and `tsconfig.base.json`'s paths map hand-lists the per-group glob fan-out. `knip.json` restates a per-package `entry` stanza for each package that gains an `*.e2e.ts` suite — byte-identical overrides that exist only because the shared `packages/*/*` stanza omits the e2e glob (an entry glob matching no files is inert, so the default stanza could carry it for every package). These lists are small today, but every new package creates another manual synchronization point.
|
||||
Package and gate inventories are repeated across TypeScript project references, package docs, CI prose, and Knip overrides. Most restate package layout, manifest data, or aggregate command contents. Each new package therefore creates avoidable synchronization points.
|
||||
|
||||
The [package hierarchy](../../implemented/architecture/2026-06-20-package-hierarchy.md) already removed several of these by hand: `scripts/publint-all.ts` now derives its list from the `packages/<group>/<pkg>` layout, and the two `tsconfig` `paths` maps collapsed to one `@deepseek-ai/dsh-*` wildcard. What remains is the inventory that cannot be globbed away — chiefly `tsconfig.build.json`'s project `references`, which TypeScript requires as an explicit array (no wildcard form).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user