From 90adf1003f39f92381819c27db2a3f56ca615ed0 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:06:54 +0800 Subject: [PATCH] docs: refine simplification audit after master merge --- docs/config-catalog.md | 2 +- docs/rfc/INDEX.md | 2 +- .../2026-06-20-unify-agent-and-session-id.md | 2 +- ...026-07-04-prune-dead-core-spine-surface.md | 23 ++++++++++--- ...12-collapse-workflow-to-foreground-core.md | 9 ++++-- ...6-07-12-prune-unconsumed-registry-modes.md | 32 ------------------- ...-12-prune-unused-skill-registry-surface.md | 27 ++++++++++++++++ ...-12-simplify-session-log-representation.md | 10 +++--- .../coding-agent/tests/keyless-smoke.e2e.ts | 2 ++ packages/core/agent/src/index.ts | 2 ++ packages/guard/repeat-tool-guard/src/index.ts | 2 ++ packages/ui/acp/src/index.ts | 9 ++++-- packages/workflow/tool-workflow/README.md | 4 +-- 13 files changed, 74 insertions(+), 52 deletions(-) delete mode 100644 docs/rfc/proposed/simplification/2026-07-12-prune-unconsumed-registry-modes.md create mode 100644 docs/rfc/proposed/simplification/2026-07-12-prune-unused-skill-registry-surface.md diff --git a/docs/config-catalog.md b/docs/config-catalog.md index e4e7ad8d3f..93f8e311f9 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -31,7 +31,7 @@ export interface AcpConfig { Depends on: `Stream` (`@agentclientprotocol/sdk`) -Source: [`packages/ui/acp/src/index.ts:248`](../packages/ui/acp/src/index.ts) +Source: [`packages/ui/acp/src/index.ts:250`](../packages/ui/acp/src/index.ts) ## `@deepseek-ai/dsh-acp-agent` diff --git a/docs/rfc/INDEX.md b/docs/rfc/INDEX.md index e4a2320f76..21ee29a58d 100644 --- a/docs/rfc/INDEX.md +++ b/docs/rfc/INDEX.md @@ -19,7 +19,7 @@ Generated by `pnpm run gen-rfc-index` from the RFC tree — never edit by hand; | [Prune dead public and result surface](proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md) | 2026-07-04 | | [Collapse workflows to the exercised foreground core](proposed/simplification/2026-07-12-collapse-workflow-to-foreground-core.md) | 2026-07-12 | | [Drop unconsumed registry events](proposed/simplification/2026-07-12-drop-unconsumed-registry-events.md) | 2026-07-12 | -| [Prune unconsumed registry modes](proposed/simplification/2026-07-12-prune-unconsumed-registry-modes.md) | 2026-07-12 | +| [Prune unused skill registry surface](proposed/simplification/2026-07-12-prune-unused-skill-registry-surface.md) | 2026-07-12 | | [Prune unused web seam fields](proposed/simplification/2026-07-12-prune-unused-web-seam-fields.md) | 2026-07-12 | | [Simplify session-log representation](proposed/simplification/2026-07-12-simplify-session-log-representation.md) | 2026-07-12 | diff --git a/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md index aaad0877c7..b3670dd9a1 100644 --- a/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md +++ b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md @@ -10,7 +10,7 @@ ACP already uses the same value for both identities. Where they diverge, stdio k The [agent-scope runtime](../../implemented/architecture/2026-07-12-agent-scope-runtime-design.md) has no reservation side tables: create and resume use one `AgentCreationTransaction`, and agent/session entries use the same final-entry collision rule. Separate ids therefore do not duplicate asynchronous liveness, rollback, or quiescence machinery. Identity unification is only an API and representation simplification: it deletes one caller-supplied id, one UUID per in-process child, and the remaining translation paths without changing the transaction lifecycle. -Session itself repeats the same fact as `Session.id` and `Session.header.id`. Valid store paths construct them equal, but the constructor does not enforce equality and production consumers choose between the two. The duplicate creates an impossible-but-representable mismatch inside the object that owns session identity. +Session itself repeats the same fact as `Session.id` and `Session.header.id`. Construction rejects a header whose id differs, so the aliases are constrained equal; the durable boundary must nevertheless validate the duplicate, and production consumers choose between its two homes. ## Proposal diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md index 58cc6d788c..cca7c57b34 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md @@ -6,7 +6,7 @@ Status: proposed Several package-root exports, result fields, and convenience methods have no production consumer. They survive because tests import internals through public entry points or because a type anticipated a caller that never arrived. Each item is small in isolation, but together they enlarge the SDK contract, generated catalogs, documentation, and regression matrix without enabling a shipped path. -The production corpus is `packages/*/*/src`, example sources/config, and runtime scripts; tests, package READMEs, generated catalogs, and RFC prose are evidence of publication but not consumers. Exact-symbol searches produce the following inventory: +The production corpus is `packages/*/*/src`, example sources/config, and runtime scripts. Tests, package READMEs, and RFC prose are evidence of publication but not fixed callers. `cordis_inspect` makes `packages/cordis/tool-cordis/src/api-catalog.ts` model-visible, and `cordis_mount` can invoke injected services through guarded real-service proxies, so catalogued service methods and returned shapes are a genuine dynamic product surface. The table therefore distinguishes absence of a fixed repository caller from unreachability: rows touching catalogued vocabulary intentionally contract what model-written mounts can discover and call, while package-root implementation helpers are not reached through that service façade. Exact-symbol searches produce the following inventory: | Surface | Production evidence | Simplification | | --- | --- | --- | @@ -15,17 +15,28 @@ The production corpus is `packages/*/*/src`, example sources/config, and runtime | `ReactLoopAgent` root export | Outside-package named imports are tests; production programs against `Agent` and creates/resumes through `ctx.agents`. | Return/interface-type `Agent` and make the concrete loop class package-internal; keep the deliberate synchronous config-only `AgentLoop.create()` path. | | `workflow-workerthread` protocol/runtime/session re-exports and named `WorkerWorkflowEngine` | Every package-name consumer uses the default engine; the workflow RFC already defines the worker wire protocol as private. | Keep the default plugin class/config contract; drop the duplicate named class export and keep protocol modules source-private. | | `code-runtime-worker` protocol/bootstrap re-exports | Outside-package production/e2e consumers use `WorkerCodeRuntime` and config, not `BootstrapPort`, `PatchableStream`, or worker message/boot types. | Keep the runtime class/config contract and make its wire/bootstrap vocabulary source-private. | +| ACP translation/presenter root exports | `agentOptions`, `streamSessionEventUpdate`, `todosToPlan`, `ToolPresenter`, `nullToolPresenter`, and `TerminalRendering` have only same-file or ACP-test consumers; the sole outside-package production consumer mounts the plugin namespace. | Keep `name`, `inject`, `Config`, `AcpConfig`, and `apply`; make translation/presentation helpers source-private and test them in-package. | | `providerWording` and `completedTurnPrefix` root exports | Each has one same-package production caller; only the balanced-prefix helper has a same-package white-box test. | Make them source-private and test provider behavior. | | `depthOf`, `SubagentDepthError`, `SENSITIVE_ENV_PATTERN`, `waitForExit`, and `exitsWithin` root exports | Production subagent backends consume the in-process runner and subprocess construction/disposal helpers, not these enforcement/test internals. | Keep depth/environment/exit behavior but make the helpers and error/regex source-private; test through spawn and disposal. | -| `PersistenceCoordinator.inits`, backend `inits` accessors, `seedCoversPrefix`, and `assertSerializable` | The accessors exist for white-box tests; the helpers have no outside production importer. | Observe initialization through `session/flush` and internalize the helpers. Keep both backends, `SessionHeader`, and SQLite's version contract. | -| `LlmService.models()` | Tests/docs only; production resolves a configured adapter directly. | Delete the convenience while keeping both LLM adapters. | +| `PersistenceCoordinator.inits`, backend `inits` accessors, `seedCoversPrefix`, and `assertSerializable` | The accessors exist for white-box tests; `seedCoversPrefix` has no outside production importer; `assertSerializable` has no production caller and duplicates the coordinator append boundary's lossless snapshot. | Observe initialization through `session/flush`, make `seedCoversPrefix` source-private, and delete `assertSerializable`. Keep both backends, `SessionHeader`, and SQLite's version contract. | | `LlmError.status` and replay status | Adapters/replay populate it, but production branches on stable error code/message and never reads raw status. | Remove the unread field and replay plumbing while preserving error classification. | | `BlockAssembler.push()` return value | Both production callers ignore the returned completed block. | Return `void`; keep the deliberately public `blocks()`/`message()` contract. | -| `compactRegion`'s separate `session` argument | The sole production caller passes the same object already present as `agent.session`; the API permits an incoherent pair. | Use `agent.session` as the one source of truth. | +| `compactRegion`'s separate `session` argument | The fixed caller passes the same object already present as `agent.session`; the model-visible mount API can also call the method, but accepting two identities permits a mounted plugin to provide an incoherent pair. | Keep the manual-region seam while deliberately narrowing it to `agent.session` as the one source of truth. | | `CompactionResult.startSeq`, `summarySeq`, `endSeq`, and `summary` | The production consumer reads only shadowed range/seq/token accounting; the durable log owns summary and event identity. | Remove the four result echoes while keeping both shared transcript renderers. | | `BasicCompactService` estimation/summarization visibility | No outside production caller invokes the five methods; the implemented RFC names only `estimateContentTokens()` and `summarize()` as subclass hooks. | Make those two `protected` and the three orchestration-only estimators private. | | `CodeLogEntry.source`/`level` and `RunCodeMeta.dispatches` | Every production consumer maps logs to text; no presenter/model path reads the other fields or the persisted dispatch count. | Make code-runtime logs strings (or text-only entries) and remove result-meta dispatch plumbing; keep the local counter that mints deterministic dispatch ids. | | `ToolNotFoundError.toolName`, `SystemPrompt.config`, and `BashTask.command` | Each stored public value has no production reader. | Drop the unread field while retaining error messages, resolved configuration behavior, and task lifecycle. | +| Backend package-root implementation helpers | The exact inventory below is called only through relative same-package imports. Production namespace imports mount the retained plugin contract without reading these properties; named root consumers are tests. | Retain each adapter/provider/service and its config/error contract; stop exporting the listed helper functions/constants at package roots. | +| Consumer package-root implementation helpers | The exact inventory below has only same-package production callers. Production namespace imports mount plugin contracts without reading helper properties; named root consumers are tests. | Retain plugin contracts and stable error codes; move tests to package-local modules or public behavior and stop exporting the listed helpers at package roots. | + +### Grouped helper-export inventory + +- `dsh-llm-deepseek`: `httpErrorCode`, `serializeMessages`, `serializeRequest`, `DONE`, `parseSse`, `mapFinishReason`, `mapUsage`, and `translate`; `dsh-llm-pi-ai`: `buildModel`, `mapStopReason`, `mapUsage`, `toPiContext`, and `toStreamChunks`. +- `dsh-bash-local`: `DEFAULT_GRACE_MS`, `ENV_OVERRIDES`, `killGroup`, `OutputCollector`, and `runBash`; `dsh-bash-sandbox`: `shellQuote`, `classifyDenial`, and `classifyRunnerFailure`; `dsh-sandbox-local`: `bwrapProfileArgs`, `landlockProfileArgs`, and `seatbeltProfileArgs`. The public mutable test-injection fields and their types are outside this proposal. +- `dsh-fs-local`: `applyLiteralEdit`, `listDirectory`, `probe`, `readForEdit`, `readTextForDiff`, `readWholeText`, `resolveLocalTarget`, `restoreLineEndings`, `streamWholeText`, and `writeFileAtomic`. +- `dsh-web-fetch-local`: `classifyContentType`, `decoderForCharset`, `isSameOrigin`, `parseCharset`, and `validateFetchUrl`; `dsh-web-search-exa`: `mapExaResponse` and `mapExaResult`; `dsh-web-search-deepseek`: `citationSnippets` and `mapAnthropicResponse`; `dsh-web-search-perplexity`: `mapPerplexityResponse` and `mapPerplexityResult`. +- `dsh-tool-fs`: `READ_LIMIT`, `STREAM_MIN_SIZE`, `READ_MAX_BYTES`, `READ_MAX_LINE_LENGTH`, `DIFF_CONTEXT`, `applyReadTool`, `parseReadArgs`, `applyWriteTool`, `formatWriteOutput`, `parseWriteArgs`, `applyEditTool`, `formatEditOutput`, `parseEditArgs`, `buildWindow`, `formatReadOutput`, `computeHunkDiffs`, and `diffsFromMeta`. +- `dsh-tool-web`: `WEB_SEARCH_MAX_RESULTS`, `applyWebSearchTool`, `formatSearchOutput`, `parseSearchArgs`, `presentSearchCall`, `applyWebFetchTool`, `formatFetchOutput`, `parseFetchArgs`, `presentFetchCall`, `renderBody`, and `htmlToMarkdown`; `dsh-timeout-policy`: `toolTimeoutResult`; `dsh-compact-basic`: `resolveConfig`; `dsh-tool-bash`: `renderResult`. ## Proposal @@ -35,6 +46,8 @@ Remove or demote every row as one bounded coordinated public-surface cleanup. Up **Keep test conveniences and self-contained results public.** Public helpers can make white-box tests convenient, self-contained result fields can look ergonomic, and future embedders might want the concrete loop or enumeration methods. Those benefits are hypothetical; today they make every implementation and document explain states that no shipped caller can observe. A real consumer can introduce the smallest contract it needs, with its ownership and failure semantics known. +**Keep every catalogued member for model-written mounts.** The self-referential toolset is a real generic consumer route, not generated-doc noise. Its value comes from an accurate, composable service surface, however, not from preserving duplicate fields or incoherent argument pairs indefinitely; each catalogued contraction above removes a fact available elsewhere on the same execution, agent, or result and updates the API reference in the same change. + ## Acceptance criteria - Exact-symbol searches show no removed surface outside this RFC and any implemented-RFC amendments. @@ -44,4 +57,4 @@ Remove or demote every row as one bounded coordinated public-surface cleanup. Up ## Risks -Most removals are compile-visible but runtime-neutral. The compaction argument cleanup deliberately forbids a session/context mismatch that no caller uses; the remaining changes can require external pre-release embedders to import less or adjust result shapes. The repository is unreleased, so carrying unsupported surface is the larger foundation cost. +Most removals are compile-visible but runtime-neutral. The compaction argument cleanup deliberately forbids a session/context mismatch while retaining the manual-region seam. External pre-release embedders and existing model-written mounts may import fewer helpers, pass fewer arguments, or receive narrower result shapes; this is an intentional product-surface contraction, not merely generated-catalog cleanup. The repository is unreleased, so carrying unsupported surface is the larger foundation cost. diff --git a/docs/rfc/proposed/simplification/2026-07-12-collapse-workflow-to-foreground-core.md b/docs/rfc/proposed/simplification/2026-07-12-collapse-workflow-to-foreground-core.md index 2540d36b91..d112aa2a3f 100644 --- a/docs/rfc/proposed/simplification/2026-07-12-collapse-workflow-to-foreground-core.md +++ b/docs/rfc/proposed/simplification/2026-07-12-collapse-workflow-to-foreground-core.md @@ -8,11 +8,15 @@ The workflow capability executes foreground JavaScript that composes subagents, The progress vocabulary is not merely unused; it cannot serve its only named future owner without redesign. `WorkflowRunInfo` contains `{id, meta}` but no parent agent, session, or tool-call identity, while the model-facing tool never exposes the run id. A global ACP listener could not route an event to the correct client session. `meta.phases` is never consulted, `phase(title)` does not validate against it, phase `detail`/`model` and agent `label`/`phase` feed only events, and `whenToUse` is validated and copied but never rendered or selected. `phase()` and `log()` still cross the worker boundary despite having no receiver. +The live handle repeats event-era data after those observers disappear. `WorkflowRun.id` has no non-event consumer, while the tool reads `run.meta.name` only to render a value it already owns as `args.meta.name`; neither belongs on the execution/cancellation handle. + +Cancellation also has two public channels for one synchronous start. `WorkflowStartRequest.signal` is passed to the worker host, while the sole production caller separately bridges the same signal to `WorkflowRun.cancel()`. Because `start()` returns the run before control can yield, there is no readiness window that requires request-time cancellation; the duplicate signal adds host listener/disarm state without closing a race. + `WorkflowError.fatal` is the same speculative branch in miniature: every production construction is fatal, `fatal: false` exists only in tests, and combinators already distinguish workflow failures with `instanceof`. ## Proposal -Keep the exercised core: `agent(prompt, { schema, model })`, `parallel`, `pipeline`, `args`, concurrency/agent caps, cancellation, bounded disposal, structured results, worker isolation, and foreground tool collection. Remove all `workflow/*` events and their event-only info/outcome types; remove `phase()`, `log()`, agent `label`/`phase`, phase declarations, `whenToUse`, and their worker messages/host observers; collapse workflow metadata to the name the tool actually uses; remove event-only run ids/meta snapshots and the synthesized agent-end ledger. Make `WorkflowError` one fatal error class without a boolean mode or `isFatalWorkflowError()` helper. +Keep the exercised core: `agent(prompt, { schema, model })`, `parallel`, `pipeline`, `args`, concurrency/agent caps, cancellation, bounded disposal, structured results, worker isolation, and foreground tool collection. Remove all `workflow/*` events and their event-only info/outcome types; remove `phase()`, `log()`, agent `label`/`phase`, phase declarations, `whenToUse`, and their worker messages/host observers; collapse workflow metadata to the name the tool actually uses; remove event-only run ids/meta snapshots and the synthesized agent-end ledger. Shrink `WorkflowRun` to `result`, `cancel()`, and `dispose()`; the tool renders the request-owned name. Remove `WorkflowStartRequest.signal` and the worker host's input-signal listener/disarm state, retaining the caller-owned bridge from its abort signal to `run.cancel()`. Make `WorkflowError` one fatal error class without a boolean mode or `isFatalWorkflowError()` helper. Amend the implemented dynamic-workflow RFC and update the seam/tool/worker READMEs, tool schema, generated catalogs and package graph, worker type-equivalence records, unit tests, and workflow snapshot/header fixtures. Progress UI work, if commissioned, starts from a correlation contract that names the parent agent/session/tool call instead of reviving this protocol unchanged. @@ -24,9 +28,10 @@ Amend the implemented dynamic-workflow RFC and update the seam/tool/worker READM - The workflow public seam contains only execution, cancellation, result, and disposal contracts with a production consumer. - No workflow event, phase/log protocol message, run-id generator, progress-only metadata, host pairing ledger, or fatal-mode branch remains. +- The run handle has no id/meta echoes, and cancellation has one holder-owned channel after synchronous `start()` returns. - Parallel/pipeline behavior, caps, cancellation quiescence, worker containment, structured output, and the model-facing workflow scenarios retain coverage. - Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass. ## Risks -This is a compile-visible contraction of the workflow DSL and event taxonomy. Existing unshipped scripts that use `phase`, `log`, labels, or descriptive metadata must shrink, and a future observer must add a better-correlated seam. The execution semantics that make workflows useful do not change. +This is a compile-visible contraction of the workflow DSL, event taxonomy, handle, and start request. Existing workflow calls that supply descriptive metadata, and scripts that use `phase`, `log`, or labels, must shrink; programmatic callers bridge their own abort source to the returned handle; and a future observer must add a better-correlated seam. The execution semantics that make workflows useful do not change. diff --git a/docs/rfc/proposed/simplification/2026-07-12-prune-unconsumed-registry-modes.md b/docs/rfc/proposed/simplification/2026-07-12-prune-unconsumed-registry-modes.md deleted file mode 100644 index c85af20f43..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-12-prune-unconsumed-registry-modes.md +++ /dev/null @@ -1,32 +0,0 @@ -# RFC: Prune unconsumed registry modes - -Status: proposed - -## Problem - -Two registries implement modes that no production registration populates. - -The skill service's embedded-runtime subsystem has zero production caller of `ctx.skills.register()`. It adds a reserved `runtime` provider name, a runtime map/rank/source, duplicate policy, a second revision in cache keys, normalization, disposers, and tests alongside the provider seam every shipped skill already uses. `SkillSummary.whenToUse` and candidate/definition `path` are parsed and copied but never read by a production consumer: the model catalog renders name/description, resource loading uses `resourceBase`, and providers own their locator. The deliberately open `metadata` extension point stays. - -`SystemPrompt` supports scope-local tool and variable providers, but every production `systemPrompt.tools()` and `systemPrompt.variable()` registration is global. Scoped assembly executes the merge path each step but finds no scoped tool/variable contribution. Scoped sections are live and stay. Supporting empty scope-local tool/variable layers adds maps plus merge/shadow/cleanup branches for combinations the product never constructs. - -## Proposal - -Remove `SkillService.register()`, `SkillRegistration`, the runtime pseudo-provider and reserved-name rules, runtime revisions/cache branches, and runtime-only source/rank normalization. Tests that need an embedded skill register a small real provider. Retain `providerRevision` as the in-flight discovery epoch, but key completed catalogs by cwd alone: every provider mutation synchronously clears the cache, and the post-await revision comparison already prevents inserting stale work. Remove `whenToUse`, `SkillCandidate.path`, and `SkillDefinition.path` from the skill contract and local-provider copies while retaining provider locator/root paths; retain `metadata`, `disableModelInvocation`, `source`, `provider`, `locator`, and `resourceBase` as either deliberate extension vocabulary or production-consumed fields. - -Keep system-prompt sections scoped, but make tool-schema and variable providers global-only and delete their scoped maps/merge logic. Fail loud if a caller attempts these unsupported scope/mode combinations instead of silently widening them. Keep both global and scoped tool guards: the interception design deliberately defines them as monotonic policy APIs. Amend the skill-system and agent-scope RFCs, READMEs, JSDoc, catalogs, and tests. - -## Alternatives considered - -**Keep all registry modes for embedders.** Runtime skill registration is convenient, and scoped variables/tool-schema fragments could support per-agent prompt customization. Neither has a shipped owner. A real embedded skill can be a provider; per-agent executable tools already belong in `agent.ctx.tools`; per-agent prompt facts can use scoped sections or context-aware global providers. - -## Acceptance criteria - -- Skill collection has one provider-backed path, a cwd-only completed-cache key, and a revision epoch only for in-flight invalidation; retained skill fields have a production reader or a recorded deliberate extension contract. -- System-prompt tools/variables have one global path, while sections retain their scoped behavior. -- Global and scoped tool guards plus structured-output commit behavior in native and Code Mode remain covered. -- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass. - -## Risks - -These are compile-visible contractions of two pre-release registries. The fail-loud rule must distinguish an unsupported scoped registration from an ordinary global registration without disturbing Cordis effect cleanup, and skill-local frontmatter parsing must continue to preserve and validate the supported metadata schema. diff --git a/docs/rfc/proposed/simplification/2026-07-12-prune-unused-skill-registry-surface.md b/docs/rfc/proposed/simplification/2026-07-12-prune-unused-skill-registry-surface.md new file mode 100644 index 0000000000..1df837e026 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-12-prune-unused-skill-registry-surface.md @@ -0,0 +1,27 @@ +# RFC: Prune unused skill registry surface + +Status: proposed + +## Problem + +The skill service's embedded-runtime subsystem has zero production caller of `ctx.skills.register()`. It adds a reserved `runtime` provider name, a runtime map/rank/source, duplicate policy, a second revision in cache keys, normalization, disposers, and tests alongside the provider seam every shipped skill already uses. `SkillSummary.whenToUse` and candidate/definition `path` are parsed and copied but never read by a production consumer: the model catalog renders name/description, resource loading uses `resourceBase`, and providers own their locator. The deliberately open `metadata` extension point stays. + +## Proposal + +Remove `SkillService.register()`, `SkillRegistration`, the runtime pseudo-provider and reserved-name rules, runtime revisions/cache branches, and runtime-only source/rank normalization. Tests that need an embedded skill register a small real provider. Retain `providerRevision` as the in-flight discovery epoch, but key completed catalogs by cwd alone: every provider mutation synchronously clears the cache, and the post-await revision comparison already prevents inserting stale work. Remove `whenToUse`, `SkillCandidate.path`, and `SkillDefinition.path` from the skill contract and local-provider copies while retaining provider locator/root paths; retain `metadata`, `disableModelInvocation`, `source`, `provider`, `locator`, and `resourceBase` as either deliberate extension vocabulary or production-consumed fields. + +Amend the skill-system RFC, README, JSDoc, catalogs, and tests. Agent-scoped system-prompt sections, tool providers, and variables are explicitly outside this proposal: final #224 made all three part of the `setup(agent.ctx)` contributor contract, so absence of a fixed in-repo scoped registration is not evidence of non-consumption. + +## Alternatives considered + +**Keep runtime skill registration for embedders.** It is a deliberate synchronous direct-definition convenience in the implemented skill RFC. A small provider wrapper can expose the same embedded data under effect-owned lifetime, but it must implement async `list()`/`get()`, carry provider identity, and accept provider duplicate semantics. The proposal chooses that one regular path over preserving a second ranking, validation, cache-invalidation, and lookup path. + +## Acceptance criteria + +- Skill collection has one provider-backed path, a cwd-only completed-cache key, and a revision epoch only for in-flight invalidation; retained skill fields have a production reader or a recorded deliberate extension contract. +- Agent-scoped prompt sections, variables, tool providers, tool guards, and structured-output commit behavior in native and Code Mode remain unchanged. +- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass. + +## Risks + +This is a compile-visible contraction of the pre-release skill registry. External programmatic `list()`/`get()` consumers lose `whenToUse` routing hints and candidate/definition `path`; the shipped model catalog never renders them, and resource resolution keeps its explicit `resourceBase` plus provider-owned opaque locator, but those fields are not observationally identical. Skill-local frontmatter parsing must continue to preserve and validate the supported metadata schema, and external providers remain able to supply embedded, filesystem, remote, or other skill sources. diff --git a/docs/rfc/proposed/simplification/2026-07-12-simplify-session-log-representation.md b/docs/rfc/proposed/simplification/2026-07-12-simplify-session-log-representation.md index d322e246df..f335afafe1 100644 --- a/docs/rfc/proposed/simplification/2026-07-12-simplify-session-log-representation.md +++ b/docs/rfc/proposed/simplification/2026-07-12-simplify-session-log-representation.md @@ -8,15 +8,15 @@ The session log maintains two representations that cost more machinery than thei `SurfaceManager` stores the same order in an array, a seq map, and mutable `prev`/`next` links. Production never reads `prev`; compact's sole `next` read is the successor of an array position. Replacement already uses `indexOf`, so the links do not make its dominant operation constant-time. A seq array with linear replacement lookup has the same asymptotic replacement cost and one representation to validate. -The request-header subsystem implements a custom system/tool delta codec and transmission-decision layer even though its contract says deltas are an encoding optimization, not a reconstructability requirement. A canonical full `request/header` only when the assembled header changes preserves replay while deleting `SystemDelta`, `ToolsDelta`, round-trip fallback, and the durable `request/header-delta` variant. Codec-only vocabulary disappears with the codec, not because its individual arms were invalid. +The request-header subsystem implements a custom system/tool delta codec and transmission-decision layer even though its contract says deltas are an encoding optimization, not a reconstructability requirement. Retaining the initial/resume full snapshot at each loop-instance boundary, then writing a canonical full `request/header` whenever that instance's assembled header changes, preserves replay while deleting `SystemDelta`, `ToolsDelta`, round-trip fallback, and the durable `request/header-delta` variant. Codec-only vocabulary disappears with the codec, not because its individual arms were invalid. This proposal deliberately retains append and replacement `sourceEventSeqs`, crash-repair provenance, and all `SessionStartSource` variants: implemented RFCs give those fields an audit/interception role that zero current readers does not overturn. ## Proposal -Make `SurfaceManager.nodes` a `readonly number[]` of event sequences and remove the public `SurfaceNode` shape. Keep the existing replacement generation; update tool-pairing balance and compaction callers to use array values/indices for predecessor, successor, and replacement ranges, removing node links and the seq-to-node map. Replace header deltas with canonical full changed-header snapshots and remove the delta codec/event/tests. +Make `SurfaceManager.nodes` a `readonly number[]` of event sequences and remove the public `SurfaceNode` shape. Keep the internal replace-generation signal; update tool-pairing balance and compaction callers to use array values/indices for predecessor, successor, and replacement ranges, removing node links and the seq-to-node map. Replace post-anchor header deltas with canonical full changed-header snapshots and remove the delta codec/event/tests; initial and resume anchors remain full snapshots even when the folded header is unchanged. -Amend the session-surface and reconstructable-request RFCs where they describe the removed encoding. Update event types/invariants, request logging/replay, persistence fixtures, generated catalogs, package docs, and snapshots. Give changed full snapshots an explicit `change` reason (or simplify the reason contract as part of the same implementation), so they are distinguishable from initial/resume/fallback writes. +Amend the session-surface and reconstructable-request RFCs where they describe the removed encoding. Update event types/invariants, request logging/replay, persistence fixtures, generated catalogs, package docs, and snapshots. Replace the codec-only `fallback` reason with an explicit `change` reason for post-anchor full snapshots, distinguishing them from the retained `initial` and `resume` anchors. `SESSION_FORMAT_VERSION` is deliberately pinned at `0`, so an old v0 log containing `request/header-delta` would otherwise pass the version check and silently lose header changes after the delta fold is deleted. Seed/load validation must reject that legacy event fail-loud at the format boundary; no compatibility fold or migration is added. @@ -26,10 +26,10 @@ Amend the session-surface and reconstructable-request RFCs where they describe t ## Acceptance criteria -- `SurfaceManager.nodes` is one ordered seq array with no `SurfaceNode`, link fields, or seq-to-node map; incremental append processing and replacement-generation invalidation remain. +- `SurfaceManager.nodes` is one ordered seq array with no `SurfaceNode`, link fields, or seq-to-node map; incremental append processing and the internal replace-generation signal remain, while the separate public `invalidate()` deletion stays owned by the dead-surface RFC. - Replaying full changed-header snapshots reconstructs exactly the same requests; no header-delta event/type/codec remains. - A v0 seed or persisted log containing legacy `request/header-delta` is rejected before replay, with coverage for JSONL and SQLite load paths. -- Current-version JSONL/SQLite replay, provenance, crash repair, compaction, snapshots, invariants, typecheck, coverage, doc-sync, build, and hygiene pass. +- New-shape v0 JSONL/SQLite replay, provenance, crash repair, compaction, snapshots, invariants, typecheck, coverage, doc-sync, build, and hygiene pass. ## Risks diff --git a/examples/coding-agent/tests/keyless-smoke.e2e.ts b/examples/coding-agent/tests/keyless-smoke.e2e.ts index c6b9f930a8..ea223bbad4 100644 --- a/examples/coding-agent/tests/keyless-smoke.e2e.ts +++ b/examples/coding-agent/tests/keyless-smoke.e2e.ts @@ -25,6 +25,8 @@ import { afterEach, describe, expect, it } from 'vitest' * product. */ +// TODO(loader-smoke-harness): extract the shared spawn/tempdir/timeout/EOF +// harness used here, code-mode-keyless-smoke, and cordis-agent's keyless smoke. // The dsh-stdio-agent bin (the demo:repl entry) and this example's cordis.yml. // The bin resolves its config-path arg from CWD; the test spawns from a temp // cwd, so we pass the example config's ABSOLUTE path. diff --git a/packages/core/agent/src/index.ts b/packages/core/agent/src/index.ts index b77e3e8a60..7864f5938c 100644 --- a/packages/core/agent/src/index.ts +++ b/packages/core/agent/src/index.ts @@ -202,6 +202,8 @@ interface FactorySlot { */ export class AgentRegistry extends Service { private store = new Map() + // TODO(agent-entry-mirror): derive exact-object checks from store.get(agent.id) + // plus entry.agent identity; this WeakMap mirrors the authoritative id map. private entries = new WeakMap() private factory: FactorySlot | undefined diff --git a/packages/guard/repeat-tool-guard/src/index.ts b/packages/guard/repeat-tool-guard/src/index.ts index 919d0541ba..6a5662d693 100644 --- a/packages/guard/repeat-tool-guard/src/index.ts +++ b/packages/guard/repeat-tool-guard/src/index.ts @@ -202,6 +202,8 @@ export function apply(ctx: Context, config: Config): void { throw new Error(`repeat-tool-guard: invalid argumentsPreviewChars ${argumentsPreviewChars} — must be an integer >= 1`) } + // TODO(agent-keyed-repeat-chain): key a WeakMap by the Agent itself; that + // removes the disposal-only status listener and cannot collide on id reuse. const chains = new Map() /** Whether a tool participates in the chain (untracked calls are transparent: they neither count nor reset). */ diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index de2dec6278..a3fdc38f97 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -107,6 +107,8 @@ export const name = 'acp' // because `initialize` advertises `loadSession: true`. `tools` lets a tool own // how its calls render (`presentCall`/`presentResult`); the bridge looks up the // definition by name and falls back to a generic presentation when absent. +// TODO(acp-session-inject): drop `sessions`; this bridge never reads +// ctx.sessions, and agent/session ownership is already behind ctx.agents. export const inject = ['agents', 'sessions', 'sessionPersistence', 'tools', 'userInteraction'] /** @@ -353,11 +355,12 @@ export function apply(ctx: Context, config: AcpConfig): void { const makePresenter = (agent?: Agent): ToolPresenter => new ToolPresenter(tools, (message) => { logger.warn(message) }, agent) // TODO(derive-acp-session-id): derive an event's id from agent.session and - // verify sessions.get(id)?.agent === agent; then this reverse map disappears. + // verify sessions.get(id)?.agent === agent; then remove this reverse map and + // SessionRecord.sessionId, whose sole read duplicates the same identity. // Live sessions keyed by id (RFC 011 multi-session), plus an agent→sessionId // reverse map so `agent/*` events (which carry only the Agent) demux in O(1). - // The two stay in lockstep: a record is added to `sessions` and the agent to - // `bySession` together, and removed together. + // The forward record and weak reverse entry are installed together; removing + // the record releases its strong Agent reference, so the WeakMap entry expires. const sessions = new Map() const bySession = new WeakMap() // Session ids whose `session/load` is mid-`resume()` (the slot is reserved diff --git a/packages/workflow/tool-workflow/README.md b/packages/workflow/tool-workflow/README.md index 44160d8fc1..a9c8900c4e 100644 --- a/packages/workflow/tool-workflow/README.md +++ b/packages/workflow/tool-workflow/README.md @@ -4,7 +4,7 @@ The model-facing **`workflow` tool**: run a JavaScript orchestration script that ## What the model sees -Two parameters: `script` (required — the full `export const meta = {...}` + body text; the tool DESCRIPTION carries the complete authoring contract: hooks, semantics, the supported schema subset) and `args` (optional JSON object exposed to the script as the `args` global; a bare list is wrapped as a field, a deliberate deviation from Claude Code's any-JSON `args` so the wire schema stays honest). The plugin also contributes a `tool:` system-prompt section carrying the usage policy — use the tool only on an explicit user ask for a workflow / large orchestration; prefer plain subagent calls for one or two delegations — per the convention that tool guidance ships with the tool plugin, never in the deployment persona. +Three parameters: `meta` (required identity data: `name`, `description`, and optional progress annotations), `script` (required plain JavaScript body — no `export const meta` statement; the tool description carries the complete authoring contract), and `args` (optional JSON object exposed to the script as the `args` global; wrap a bare list in a field so the wire schema stays honest). The plugin also contributes a `tool:` system-prompt section carrying the usage policy — use the tool only on an explicit user ask for a workflow / large orchestration; prefer plain subagent calls for one or two delegations — per the convention that tool guidance ships with the tool plugin, never in the deployment persona. ## Lifecycle @@ -12,7 +12,7 @@ Collection is SYNCHRONOUS this cut (like [`dsh-tool-subagent`](../../subagent/to ## Render intent -Decided up front (per the [render-intent RFC](../../../docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md)): a `generic` card titled `workflow: `, the name sniffed TEXTUALLY from `args.script` (presentation must be a pure function of args, so it cannot ask the engine to parse); the script text rides as `rawInput`. The result keeps the generic card. +Decided up front (per the [render-intent RFC](../../../docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md)): a `generic` card titled `workflow: `, read directly from `args.meta.name` (presentation is a pure function of args and does not ask the engine to parse); the script text rides as `rawInput`. The result keeps the generic card. ## Config