Browse Source

fix: share scope carrier across built JSON-RPC

Tianyi Cui 3 months ago
parent
commit
c6cb9f4210

+ 1 - 1
AGENTS.md

@@ -76,7 +76,7 @@ printf '%s\n' "$out" | grep -q '\[tool call\] echo({"text":"ci smoke"})'
 printf '%s\n' "$out" | grep -q '\[tool result\] ECHO: CI SMOKE'
 test -n "$(find .sessions -path '.sessions/cwd-*/main-session-*.jsonl' -type f -print -quit)"
 rm -rf .sessions
-pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts packages/workflow/workflow-workerthread/tests/built-worker.e2e.ts packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts
+pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts packages/workflow/workflow-workerthread/tests/built-worker.e2e.ts packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts
 ```
 
 `test:coverage`, not `test`, is the gating run ([why](docs/testing.md)); a sign-off counts only for commands actually run.

+ 1 - 1
docs/testing.md

@@ -25,7 +25,7 @@ An e2e assertion re-runs the command or re-reads the file externally; a keyword
 
 - A plugin shipped via `cordis.yml` needs at least one test through the REAL Loader path: hand-built `ctx.plugin({...})` mounts bypass `unwrapExports` and cannot catch a broken export shape ([postmortem 0001](postmortem/0001-acp-default-export-drops-inject.md); export-shape rules in [packages/AGENTS.md](../packages/AGENTS.md)).
 - A guard only guards if the regression actually fails it. For a plugin without `inject` (bundle/composition plugins), a Loader smoke stays green under a broken export shape — add an explicit `expect('default' in mod).toBe(false)` plus an `unwrapExports` round-trip assertion, and prove it: introduce the regression, watch red, revert.
-- "Real entry path" means the published artifact: the package `bin` points at built `lib/bin.js` under plain `node`, which tsx masks (settle races, module resolution, a swallowed load failure exiting 0). The same applies to any non-index runtime entry the built package resolves at run time (the worker-thread runtime's sibling `lib/worker.cjs`). Keep the built-artifact smokes green (`packages/ui/*/tests/built-bin.e2e.ts`, `packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts`), and assert a genuinely-missing config exits non-zero.
+- "Real entry path" means the published artifact: a package `bin` runs built `lib/bin.js` under plain `node`, exposing failures tsx masks (settle races, module resolution, swallowed load failures). The same applies to non-index runtime entries (the worker-thread sibling `lib/worker.cjs`) and singleton modules shared across bundles (`packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts`). Keep the built-artifact smokes green (`packages/ui/*/tests/built-bin.e2e.ts`, `packages/code-runtime/code-runtime-worker/tests/built-lib.e2e.ts`), and assert a genuinely-missing config exits non-zero.
 - An e2e that spawns an example from a temp cwd sets `TSX_TSCONFIG_PATH` to the repo-root tsconfig, or it silently falls back to stale built `lib/` ([examples/AGENTS.md](../examples/AGENTS.md)).
 
 ## When a snapshot test is required

+ 2 - 0
packages/ui/jsonrpc/package.json

@@ -28,6 +28,7 @@
     "@deepseek-ai/dsh-agent": "^0.0.1",
     "@deepseek-ai/dsh-llm": "^0.0.1",
     "@deepseek-ai/dsh-llm-deepseek": "^0.0.1",
+    "@deepseek-ai/dsh-scope": "^0.0.1",
     "@deepseek-ai/dsh-session": "^0.0.1",
     "@deepseek-ai/dsh-subagent": "^0.0.1",
     "cordis": "^4.0.0-rc.6"
@@ -38,6 +39,7 @@
     "@deepseek-ai/dsh-agent-core": "workspace:^",
     "@deepseek-ai/dsh-llm": "workspace:^",
     "@deepseek-ai/dsh-llm-deepseek": "workspace:^",
+    "@deepseek-ai/dsh-scope": "workspace:^",
     "@deepseek-ai/dsh-session": "workspace:^",
     "@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^",
     "@deepseek-ai/dsh-subagent": "workspace:^",

+ 121 - 0
packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts

@@ -0,0 +1,121 @@
+/**
+ * Built-artifact guard for the scope carrier shared by `dsh-subagent` and
+ * `dsh-jsonrpc`. The carrier registry is module-local, so both bundles must
+ * externalize `dsh-scope`; source-mode tests cannot expose an accidentally
+ * inlined second registry. This test runs the real `lib/index.js` bundles in a
+ * plain Node subprocess, disposes the child before settlement, and requires the
+ * SDK completion notification to retain the delegating parent.
+ */
+
+import { execFile } from 'node:child_process'
+import { existsSync } from 'node:fs'
+import { fileURLToPath } from 'node:url'
+import { promisify } from 'node:util'
+import { describe, expect, it } from 'vitest'
+
+const repoRoot = fileURLToPath(new URL('../../../../', import.meta.url))
+const jsonrpcBundle = fileURLToPath(new URL('../lib/index.js', import.meta.url))
+const execFileAsync = promisify(execFile)
+
+const builtRuntimeProbe = String.raw`
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
+import { pathToFileURL } from "node:url";
+
+const load = (path) => import(pathToFileURL(resolve(path)).href);
+const [
+  { Context },
+  agentCore,
+  { default: SubagentService },
+  { default: SessionPersistenceJsonl },
+  { HarnessSdkServer },
+  { SessionId },
+] = await Promise.all([
+  load("vendor/cordis/lib/index.js"),
+  load("packages/core/agent-core/lib/index.js"),
+  load("packages/subagent/subagent/lib/index.js"),
+  load("packages/session-persistence/session-persistence-jsonl/lib/index.js"),
+  load("packages/ui/jsonrpc/lib/index.js"),
+  load("packages/core/session/lib/index.js"),
+]);
+
+const storageRoot = await mkdtemp(join(tmpdir(), "jsonrpc-built-scope-"));
+const ctx = new Context();
+try {
+  await ctx.plugin(agentCore);
+  await ctx.plugin(SubagentService);
+  await ctx.plugin(SessionPersistenceJsonl, { root: storageRoot });
+  await new Promise((ready) => setTimeout(ready, 50));
+
+  const notifications = [];
+  const server = new HarnessSdkServer(ctx, {
+    request() { return Promise.reject(new Error("unexpected host request")); },
+    notify(method, params) { notifications.push({ method, params }); },
+  });
+  const parent = await ctx.agents.create({
+    sessionId: SessionId("built-parent"),
+    meta: { cwd: storageRoot },
+    agentOptions: { model: "test" },
+  });
+  const child = await ctx.agents.create({
+    sessionId: SessionId("built-child"),
+    meta: { cwd: storageRoot, parentSession: SessionId("built-parent") },
+    agentOptions: { model: "test" },
+  });
+  const result = Promise.withResolvers();
+  const unregister = ctx.subagents.registerProvider({
+    name: "built-local",
+    capabilities: { outputSchema: false, depthLimit: false, toolFilter: false, persona: false },
+    inheritsParentContext: false,
+    start() {
+      return Promise.resolve({
+        id: child.agent.id,
+        result: result.promise,
+        dispose() { return Promise.resolve(); },
+      });
+    },
+  });
+  const run = await ctx.subagents.start("built-local", {
+    parent: parent.agent,
+    prompt: [],
+    signal: new AbortController().signal,
+  });
+  await child.dispose();
+  result.resolve({ output: [], stopReason: "completed" });
+  await run.result;
+  await Promise.resolve();
+
+  console.log(JSON.stringify(notifications.filter(({ method }) => method === "subagent.finished")));
+  await run.dispose();
+  unregister();
+  await parent.dispose();
+  await server.shutdown();
+} finally {
+  await ctx.fiber.dispose();
+  await rm(storageRoot, { recursive: true, force: true });
+}
+`
+
+describe.skipIf(!existsSync(jsonrpcBundle))('dsh-jsonrpc BUILT scope carrier', () => {
+  it('preserves parent-scoped completion after child disposal', async () => {
+    const { stdout, stderr } = await execFileAsync(process.execPath, ['--input-type=module', '-e', builtRuntimeProbe], {
+      cwd: repoRoot,
+      timeout: 15_000,
+    })
+
+    expect(stderr).not.toContain('listener threw')
+    expect(JSON.parse(stdout) as unknown).toEqual([{
+      method: 'subagent.finished',
+      params: {
+        provider: 'built-local',
+        agentId: 'built-child',
+        parentSessionId: 'built-parent',
+        childSessionId: 'built-child',
+        status: 'ok',
+        stopReason: 'completed',
+        lastAssistantMessage: [],
+      },
+    }])
+  })
+})

+ 3 - 0
pnpm-lock.yaml

@@ -1288,6 +1288,9 @@ importers:
       '@deepseek-ai/dsh-llm-deepseek':
         specifier: workspace:^
         version: link:../../llm/llm-deepseek
+      '@deepseek-ai/dsh-scope':
+        specifier: workspace:^
+        version: link:../../core/scope
       '@deepseek-ai/dsh-session':
         specifier: workspace:^
         version: link:../../core/session

+ 1 - 0
scripts/run-gates.ts

@@ -332,6 +332,7 @@ function builtBinSmokeGate(): Gate {
     'vitest.e2e.config.ts',
     'packages/ui/stdio-agent/tests/built-bin.e2e.ts',
     'packages/ui/acp-agent/tests/built-bin.e2e.ts',
+    'packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts',
     // The worker-entry packages' built bundles: the only automated proof
     // that lib/index.js resolves its sibling lib/worker.cjs under plain node
     // (the e2e lane runs unbuilt, so these files self-skip there).