Add in-process subagent backends: spawn (fresh) and fork (seeded)
The second PR of the subagent seam: the two in-process backends that run a
child agent on the same cordis context, reusing the agent factory's quiescent
AgentHandle teardown. Both register on ctx.subagents (PR1's named-provider
registry) and share one run driver.
- dsh-subagent-spawn: a FRESH child via ctx.agents.create — own session, the
parent's model by default (overridable), zero inherited conversation. Also
exports the shared in-process run driver (startInProcessRun): mint ids, stamp
cwd/parentSession-lineage/depth, drive the one-shot (send → whenIdle), read
the last assistant/message + turn/end reason, dispose to quiescence.
- dsh-subagent-fork: a child SEEDED with the parent's balanced completed-turn
prefix (the log up to and including its last turn/end), so the child inherits
context. The in-flight unbalanced turn is excluded — a raw seed would fail the
invariants replay. Proven: a regression test goes red if the boundary seeds
the open turn.
- Seam extension: CreateAgentOptions.seed, threaded through AgentLoop.createAgent
→ ctx.sessions.prepare({ seed }) (the primitive resume already used). This is
the fork-lineage path the TODO(sub-agents) markers anticipated.
- Depth: a merge-extensible AgentOptions.subagentDepth (0 top-level, parent+1 for
a child); the depthLimit capability refuses a spawn past request.maxDepth.
Tests: real-loop unit tests for both backends (mock MODEL only, real loop +
invariants), a multi-subagent test (one parent drives a fork AND a spawn child
then keeps working), and a with-key e2e (a real parent delegates via the
`subagent` tool to a real child that writes a file on disk — world-verified).
100% per-file coverage. The coding-agent demo wires the spawn backend + tool.
Snapshot coverage of nested agents is deferred to a stacked follow-up
(TODO(subagent-snapshots)): dsh-llm-replay is a single global positional cursor
that cannot route calls to a parent vs. a child on one context. Recorded in the
RFC's deferrals and a new AGENTS.md rule: designing a subsystem must design its
test infrastructure END TO END up front, verifying the snapshot/e2e harness can
express the new shape — a gap this plan hit.
This commit is contained in:
@@ -207,6 +207,7 @@ Dev/test/demo run **unbuilt** via tsx + the `paths` map in the root `tsconfig.js
|
||||
- **Tests**: vitest, colocated under `packages/<group>/<pkg>/tests/*.spec.ts`. Every registry needs an HMR-safety test (dispose the contributing fiber, assert cleanup). **Excessive tests are welcome** — when in doubt, write the test; err on the side of covering edge cases, error paths, event ordering, and concurrency races even if they seem unlikely. Review findings get regression tests (see `packages/core/agent-loop/tests/review-fixes.spec.ts`). The same generosity applies to **real-API (with-key) e2e tests — inference is cheap here (we are DeepSeek), so do not ration them**: cover the agent's real flows (a real prompt that writes a file, multi-turn, tool use, cancellation) and run them frequently while developing, especially cheap **smoke tests** that boot the real example and check the world. A green mock/no-key suite proves the plumbing, not the product — the with-key smoke test is what catches "green units, broken product". See § Secrets / .env for the with-key policy and why self-skip is a CI accommodation, not a verdict that real-API tests are expensive.
|
||||
- **Prefer the REAL implementation over a mock/stand-in in tests.** When the genuine collaborator is available in the repo, wire it up instead of hand-rolling a fake — a test that registers an inline `defineTool({ name: 'bash', … })` to stand in for `dsh-tool-bash` proves the *bridge* moves bytes but not that the *shipping tool* renders the way the test asserts; the two drift and the test passes while the product is wrong. Mock only the genuinely expensive/non-deterministic boundary (the LLM adapter, the network, the clock) and keep everything downstream real: a bridge tool-call test runs the scripted mock MODEL but the REAL tool + REAL executor (e.g. `makeBridgeHarness({ withBash: true })` plugs `dsh-bash-local` + `dsh-tool-bash` and runs an actual `echo`), so it verifies the actual `presentCall`/`presentResult` an editor sees. This is the unit-test echo of "verify the world, not a synthetic stand-in" (see § Defensive patterns) — a fake you wrote will agree with whatever you assumed; the real thing won't.
|
||||
- **A change that affects the editor-facing transcript or end-to-end agent UX needs a snapshot test (or an explicit note in the PR why none applies).** The snapshot tier (`examples/*/tests/**/*.snapshot.ts`, `pnpm run test:snapshot`) boots the real example subprocess, replays a recorded session JSONL deterministically (keyless), and diffs the normalized stdout transcript + re-persisted session log against committed goldens — the full-transcript regression net that mock-level unit tests structurally cannot be (it is what catches a bridge-translation or loop-structure regression that leaves every unit green). When you change the ACP bridge, the agent loop's observable output, tool presentation, or anything an editor renders, add or update a scenario under `examples/acp-agent/tests/snapshots/` and re-record with `pnpm run test:snapshot:record`. Reviewing the golden diff is part of the review. The rule is scoped to transcript/UX-affecting changes — a pure internal refactor with no observable-output change does not need one, but say so. See [docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md](docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md).
|
||||
- **Designing a new subsystem includes designing its test infrastructure — END TO END, up front, as part of the same plan.** When you introduce a new capability seam, a new agent-lifecycle shape, or anything that produces an observable transcript (a new tool family, a subagent transport, a new UI surface), the plan must name how it will be covered at EVERY tier it touches — unit, real-API e2e, AND the full-transcript snapshot tier — and, critically, must check that the existing test infrastructure can actually express that coverage. Do not assume a snapshot/e2e harness built for one shape (e.g. a single top-level ACP session) transparently supports a new shape (e.g. a parent agent driving nested child agents): verify it, and if it cannot, the harness extension is in-scope work to plan and schedule, not a detail to discover mid-implementation. This rule exists because a real plan under-scoped exactly this: the subagent backends were planned with unit + e2e coverage but the snapshot tier turned out to assume one session per process (`dsh-llm-replay`'s single positional cursor, single-file harvest), so nested-agent snapshot coverage became unplanned net-new infrastructure (`TODO(subagent-snapshots)`). The cost of finding that during design is a paragraph; the cost of finding it mid-build is a re-plan. When the harness gap is large enough to be its own reviewable unit, schedule it as a dedicated stacked follow-up with its own RFC — but SAY SO in the originating plan, with the gap named, rather than letting it surface as a surprise.
|
||||
|
||||
## Defensive patterns (hard-won)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user