Commit Graph

2 Commits

Author SHA1 Message Date
Tianyi Cui 90a19f072d docs(acp,rfc): fix stale ownership wording + propose unifying agent/session id (review)
Review follow-ups on the bash owner-token PR:

- packages/acp/README.md still described task isolation in object-identity terms
  ("records each background task's owning agent", "a different agent"). Rewrite
  to the session-token model: ownership is by `session.header.id`, stored on the
  executor's task, so a different Agent object on the same session may access it
  and ownership survives a tool-bash HMR reload.

- The reviewer flagged that the notice routes by `session.header.id` while the
  registry only enforces unique `agent.id`, so a programmatic caller could
  register two agents sharing a session token and mis-route a notice (not
  reachable via ACP). Rather than bolt a session-id invariant onto the generic
  registry, add a proposed RFC (2026-06-20-unify-agent-and-session-id) to remove
  the precondition by construction — an agent IS its session, one id — with a
  full risks discussion (forecloses multi-session-actor / fork futures, makes the
  config resume-or-create policy load-bearing, migration churn). The actual
  unification ships as its own Codex-converged PR. Cross-linked from the
  agent-lifecycle RFC's seam-precondition note.

- Reframe the tool-bash module-doc ownership paragraph to current-state (per the
  new AGENTS.md doc convention): contrast storing the token on the executor vs
  in the plugin as a standing rationale, not as "closing the old gap".
2026-06-20 13:38:48 +08:00
Tianyi Cui b58f1dd5c8 refactor(tool-bash): own background tasks by session token, not a plugin-local Map
Delete the `taskOwner: Map<string, Agent>` entirely — it served two roles
(access control AND holding a live Agent for completion notices), both now
stateless:

- Access control: `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to
  the caller's token (`exec.agent?.session.header.id`) with `!== undefined`
  semantics (an empty-string token is still a real owner). The owner is stamped
  at spawn via `resolve({ …, owner })`. Ownership now lives on the task in the
  executor, so it SURVIVES a tool-bash HMR reload — closing the old
  XXX(tool-bash-owner-hmr) gap.
- Completion notice: `onTaskDone` reads `ctx.bash.ownerOf(task.id)` and finds
  the live agent by scanning `ctx.get('agents')?.list()` for a matching
  `session.header.id` (read via `ctx.get` — the listener runs on the bash
  fiber, a foreign fiber, where the `ctx.agents` proxy would throw). No
  registry / owner gone → drop the notice cleanly.

Token is `session.header.id` (NOT `session.id`): every other subsystem keys off
the header id, and the test fakes populate only `session.header.id`, so reading
`session.id` would make every fake unowned and pass the isolation tests for the
wrong reason.

Tests give A and B DISTINCT real session tokens (a same-token-different-Agent
case is now ALLOWED — identity no longer matters); the HMR test inverts to
assert ownership SURVIVES a tool-bash reload; a new test covers the
owner-agent-gone-before-completion drop. Migrates the agent-lifecycle RFC
proposed->implemented (recording all three seams + the session-id-uniqueness
precondition) and updates the tool-bash README + the now-implemented RFC's
cross-links.
2026-06-20 08:14:27 +08:00