Merge remote-tracking branch 'origin/worktree-agent-scope-design' into codex/pr224-simplification-audit
# Conflicts: # docs/config-catalog.md # docs/cordis-catalog/services.md
This commit is contained in:
@@ -4,30 +4,30 @@ Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The agent factory carries two ids for what every supported ownership path treats as one live agent/session pair: `agentId`, the `AgentRegistry` handle, and `sessionId`, the event-sourced/persisted-log identity. `CreateAgentOptions` takes both; `ResumeAgentOptions` takes `agentId` plus `resumeSessionId`; in-process subagents mint two independent UUIDs despite recording lineage separately.
|
||||
The agent factory carries two ids for each live agent/session pair: `agentId`, the `AgentRegistry` routing handle, and `sessionId`, the event-sourced/persisted-log identity. `CreateAgentOptions` takes both; `ResumeAgentOptions` takes `agentId` plus `resumeSessionId`; in-process subagents mint two independent UUIDs despite recording lineage separately.
|
||||
|
||||
ACP already uses the same value for both identities. Where they diverge, consumers maintain translations rather than use the distinction: stdio keeps `labelBySession` solely to recover an agent label from session events, ACP keeps reverse ownership state, and hooks expose both values for authors to reconcile. No production path reattaches one stable actor id to several sessions or drives one session through several agent ids.
|
||||
ACP already uses the same value for both identities. Where they diverge, stdio keeps `labelBySession` solely to recover an agent label from session events, and hooks expose both values for authors to reconcile. No production path reattaches one live agent object to several sessions or drives one session through several agent ids.
|
||||
|
||||
The [agent-scope design](../../implemented/architecture/2026-07-08-agent-scope-contexts.md) makes the cost concrete. The `AgentLoop` factory reserves agent ids and session ids independently during asynchronous setup, with paired rollback paths, even though successful creation always publishes one pair; the registry and store then recheck live publication. PR #224 correctly closes the former duplicate-session ownership hole by reserving and rechecking both ids, so identity unification is no longer a correctness fix; it is a way to delete the second reservation/index/translation system.
|
||||
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.
|
||||
|
||||
## Proposal
|
||||
|
||||
Make an agent's registry id equal its session id. `CreateAgentOptions` accepts one id used for both registration and session creation; resume registers the agent under the resumed session id; subagent creation mints one combined id; Session keeps one identity home by deriving `id` from `header.id` or removing the alias. Replace the two reservation sets and rollback branches with one combined identity reservation, and remove maps/fields whose sole job is translating between the ids.
|
||||
Make an agent's registry id equal its session id. `CreateAgentOptions` accepts one id used for both final registry entries; resume registers the agent under the resumed session id; subagent creation mints one combined id; Session keeps one identity home by deriving `id` from `header.id` or removing the alias. Keep the existing creation transaction, final-entry collision checks, and exact-entry detach semantics; remove only maps and fields whose sole job is translating between the ids.
|
||||
|
||||
The config-driven path must first settle its currently hidden resume-or-create policy. Today it uses a stable agent label and fresh UUID-suffixed session id to avoid colliding with a durable log on the next run. Under unification it must deliberately resume the fixed id, mint a fresh combined id, or expose an explicit policy; implementation must not pick silently.
|
||||
|
||||
After stdio's translation map disappears, rerun the consumer search for `agent/created` and `agent/disposed`. If it is empty, remove those notifications together with `AgentRegistry.announced`/`announce()` and their publication rollback machinery. PR #224 deliberately hardened those lifecycle semantics, so this follow-on removal is conditional on proving that identity unification eliminated their last owner.
|
||||
`agent/created` and `agent/disposed` remain outside this proposal. They are paired publication lifecycle events, not identity aliases; any later consumer-free removal belongs in the dedicated [registry-event simplification](./2026-07-12-drop-unconsumed-registry-events.md) after a fresh search.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Keep separate actor and session identities.** This leaves room for handoff, one actor traversing many logs, or many actors adopting one log. None is supported today. If that product direction arrives, it deserves an explicit actor/handoff seam with ownership semantics rather than two ids that happen to differ in a few constructors.
|
||||
**Keep separate routing and log identities.** The config-driven loop uses a stable configured agent id with a fresh UUID session on each fresh process start. That is a real use of the distinction: a stable routing/display label plus a new durable conversation. Unification can proceed only after choosing whether this path resumes a fixed identity, mints a combined per-run identity, or exposes the policy explicitly. If the stable label is a required product contract, reject this proposal rather than hiding it in another map.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- Agent create/resume and subagent creation carry one identity; `Session` stores it in one place.
|
||||
- The factory keeps one in-flight reservation/rollback path while preserving PR #224's duplicate and quiescence guarantees.
|
||||
- The existing creation transaction keeps final-entry collision, exact-entry detach, rollback, and quiescence guarantees without adding identity-specific lifecycle state.
|
||||
- ACP, stdio, hooks, bash ownership, persistence, and lineage need no agent/session id translation.
|
||||
- The config-driven resume-or-create policy is explicit and covered across a durable restart.
|
||||
- `agent/created`/`agent/disposed` are removed only if a post-change production search finds no listener; otherwise they and their publication semantics stay.
|
||||
@@ -35,4 +35,4 @@ After stdio's translation map disappears, rerun the consumer search for `agent/c
|
||||
|
||||
## Risks
|
||||
|
||||
This forecloses latent multi-session-actor and session-handoff designs, makes persisted client-chosen session identity the registry identity, and touches every factory fixture. The config restart decision is blocking, not mechanical. If separate actor identity becomes a real requirement, reject this RFC and retain PR #224's already-correct dual reservation system.
|
||||
This forecloses latent multi-session-actor and session-handoff designs, makes persisted client-chosen session identity the registry identity, and touches every factory fixture. The config restart decision is blocking, not mechanical. If separate routing identity is a real requirement, reject this RFC and retain the current caller-supplied pair plus final-entry arbitration.
|
||||
|
||||
@@ -27,8 +27,6 @@ The production corpus is `packages/*/*/src`, example sources/config, and runtime
|
||||
| `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. |
|
||||
|
||||
The earlier version of this RFC also named `runLoop`, `Inbox`, and `InboxMessage`; the agent-scope branch has already made those package-internal, so they are no longer proposed work.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove or demote every row as one bounded coordinated public-surface cleanup. Update package READMEs, JSDoc, generated API/event catalogs, type-equivalence records, exports maps where needed, and tests so they exercise the owning public seam instead of preserving test-only entry points. Do not collapse any capability seam, LLM adapter, persistence backend, or lifecycle quiescence contract.
|
||||
|
||||
@@ -4,7 +4,7 @@ Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The workflow capability executes foreground JavaScript that composes subagents, but it also carries an unconsumed progress-observation system. No production listener subscribes to any of the six `workflow/*` events; listeners exist only in workflow tests. Nevertheless the seam defines run/phase/agent outcome snapshots, the worker sends phase/log/agent lifecycle protocol messages, the host clones payloads and keeps a `liveAgents` pairing ledger, and the engine maintains run ids solely to correlate those notifications.
|
||||
The workflow capability executes foreground JavaScript that composes subagents, but it also carries an unconsumed progress-observation system. No production listener subscribes to any of the six `workflow/*` events; listeners exist only in workflow tests. Nevertheless the seam defines run/phase/agent outcome payloads, the worker sends phase/log/agent lifecycle protocol messages, the host forwards them through a `liveAgents` pairing ledger, and the engine maintains run ids solely to correlate those notifications.
|
||||
|
||||
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.
|
||||
|
||||
@@ -18,7 +18,7 @@ Amend the implemented dynamic-workflow RFC and update the seam/tool/worker READM
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Keep the prebuilt observation vocabulary for a future UI.** The current shape resembles Claude Code dynamic-workflow metadata, and PR #233 deliberately added host pairing state and synthesized agent-end events so observers would see balanced lifecycles; this proposal reopens that recent choice rather than treating the machinery as accidental. Removing it gives up compatibility-by-shape and makes progress UI a new design task, but the existing shape still lacks routable ownership, so its hardened pairing cannot make the named ACP owner viable without redesign.
|
||||
**Keep the prebuilt observation vocabulary for a future UI.** The current shape resembles Claude Code dynamic-workflow metadata, and the host deliberately pairs each forwarded agent start with either the worker's end or a synthesized terminal end. Removing it gives up compatibility-by-shape and makes progress UI a new design task, but the existing payloads still lack routable ownership, so balanced lifecycles alone cannot make the named ACP owner viable without redesign.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ Status: proposed
|
||||
|
||||
Four registry notifications are produced but have no production listener. The generated producer/consumer matrix and exact event-name searches find only declarations, emit sites, invariant metadata, tests, generated catalogs, and prose for `tools/change`, `system-prompt/change`, `skill/provider-added`, and `skill/provider-removed`.
|
||||
|
||||
No shipped path uses these signals for invalidation: request assembly deliberately reruns for every step, tool/system-prompt membership is now agent-scoped, and skill discovery reads providers on demand. PR #224 also makes the payloadless tool/system-prompt notices less coherent because a change may be scope-local but the event cannot identify that scope.
|
||||
No shipped path uses these signals for invalidation: request assembly deliberately reruns for every step, tool/system-prompt membership may be agent-scoped, and skill discovery reads providers on demand. The payloadless tool/system-prompt notices are also insufficient for a scoped observer because a change may be local to one agent but the event cannot identify that scope.
|
||||
|
||||
Earlier registry work retained tool/system-prompt notifications as low-cost hooks for a hypothetical live UI even while the equivalent LLM and web notifications were removed. The new evidence is that no owner has appeared, per-step assembly needs no signal, and scope-local membership has made the old payload insufficient for that hypothetical owner. This proposal does not include `subagent/provider-added`/`removed`, which `tool-subagent` consumes to tolerate concurrent sibling-plugin loading.
|
||||
|
||||
|
||||
@@ -8,13 +8,13 @@ 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.
|
||||
|
||||
The agent-scope work generalized system-prompt tool and variable providers to scope-local registration, 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 and protections are live and stay. Supporting empty scope-local tool/variable layers adds maps plus merge/shadow/cleanup branches for combinations the product never constructs.
|
||||
`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 and contribution-level `ownerFinal` behavior 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/protections 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 agent-scope/interception design deliberately defines them as owner-final policy APIs. Amend the skill-system and agent-scope RFCs, READMEs, JSDoc, catalogs, and tests.
|
||||
Keep system-prompt sections and contribution-level finality 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
|
||||
|
||||
@@ -23,7 +23,7 @@ Keep system-prompt sections/protections scoped, but make tool-schema and variabl
|
||||
## 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; sections/protections retain their scoped behavior.
|
||||
- System-prompt tools/variables have one global path; sections and contribution-level finality retain their scoped behavior.
|
||||
- Global and scoped tool guards, native finality, and Code Mode finality behavior remain covered.
|
||||
- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user