Commit Graph
3 Commits
Author SHA1 Message Date
Tianyi Cui 7c168fca34 fix(acp): address Codex review — strict ctx.get, correct fiber-ownership doc + test
- AgentLoop.resume uses `this.ctx.get('sessionPersistence')` (strict) instead
  of the `, false` overload: still topology-independent, but an inactive/
  absent backend reads as undefined (rejected by the existing guard) rather
  than being handed back mid-teardown.
- Correct the bridge teardown comment: an ACP-created agent's registry entry
  binds to the BRIDGE fiber (the factory is reached through the bridge's
  traceable proxy, so AgentLoop.start's `this.ctx.effect` registration uses the
  caller context), not the AgentLoop fiber — so an ACP-only HMR dispose
  reclaims it. Add a regression test pinning that ownership.
- Sync the ctx.get guidance in the post-mortem, packages/AGENTS.md, and the
  dsh-code-review skill to the strict form.
2026-06-18 03:48:33 +08:00
Tianyi Cui 86ec067bff Merge remote-tracking branch 'origin/master' into feat/acp-2-bridge
# Conflicts:
#	.agents/skills/dsh-code-review/SKILL.md
#	AGENTS.md
#	docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md
2026-06-18 03:31:04 +08:00
Tianyi Cui 6d37b6c33d fix(acp): server crashed on connect — drop export default, read optional service cwd-independently
Two independent bugs made the ACP server crash the moment an editor (Zed)
connected, despite 178 green unit tests at 100% coverage:

1. `session/new` threw `cannot get property "agents" without inject`. Root
   cause: a stray `export default apply` made the cordis Loader's
   `unwrapExports` (`exports.default ?? exports`) collapse the module to the
   bare `apply` function, discarding the sibling `inject`/`name`/`Config`
   named exports. The plugin fiber was built with empty `inject`, so every
   `ctx.<service>` read in `apply` threw at load. Fix: remove the default
   export so the Loader uses the namespace.

2. `session/load` threw `cannot get property "sessionPersistence" without
   inject`. `AgentLoop.resume` read `this.ctx.sessionPersistence` (a service
   it deliberately does NOT inject); the property proxy's ancestor-only fiber
   walk fails through the bridge's traceable shadow. Fix: read it via
   `this.ctx.get('sessionPersistence', false)`, the topology-independent
   global-store lookup.

Why the suite missed both: every test mounted the plugin by hand
(`ctx.plugin({name,inject,apply})`), bypassing `unwrapExports` entirely, and
the only test driving these RPCs was key-gated (skipped in CI). Added a no-key
`session/new` e2e that boots the real example through the real Loader — it
fails loudly on bug #1 without an API key. Set `TSX_TSCONFIG_PATH` in the e2e
spawn so the subprocess resolves workspace `paths` from a temp cwd (it was
silently falling back to a stale built `lib/`).

Docs: post-mortem 0001; AGENTS.md "line coverage is not behavior coverage" +
with-key/smoke-test philosophy; packages/AGENTS.md plugin-export-shape and
ctx.get rules; dsh-code-review SKILL checks.
2026-06-18 03:12:37 +08:00