Browse Source

fix(agent-presets): keep the code preset on one plane, and check that

`code` still carried `bash-env` behind its own `isolate` realm and its own
`tool-subagent-report` row — the two the other three presets had already given
back to the host. It was added a layer above the fix, so the rebase carried it
forward untouched, and the shipped deployment ran a preset whose sessions get
no `DSH_WEB_URL` in their shell and hand every subagent a second `report`
registration on the host registry.

Nothing caught it. A tool-catalog assertion cannot: neither row contributes a
tool. The web lane cannot: no scenario composes `code` beside another preset,
which is when the second `report` throws. The presets are near-copies of one
another, so "fixed in three of four" is the shape this failure takes, and it
will take it again.

So the invariant is checked rather than described. `verify-cordis-config` now
rejects any shipped preset row that is also active on the host plane, which is
the property both defects violated: a row active on both planes is mounted once
per process and once per session, and what that costs depends on the row — a
provider behind an `isolate` realm shadows the host's for its own consumers, so
a host contributor reaches nobody; a row registering into a host singleton
registers once per live session, so the second collides.
Yichen Jiang 2 months ago
parent
commit
ef089ed6ec
2 changed files with 79 additions and 19 deletions
  1. 9 19
      apps/cli/config/agent-presets/code/agent.cordis.yml
  2. 70 0
      scripts/verify-cordis-config.ts

+ 9 - 19
apps/cli/config/agent-presets/code/agent.cordis.yml

@@ -41,22 +41,15 @@
 
 # ── shell ───────────────────────────────────────────────────────────────────
 
-# `tool-bash` reads as a tool but provides the `bashEnv` service, so it needs a
-# realm like any other provider. The executor behind it (`bash-sandbox`) stays
-# in the host composition, where the sandbox policy owns it.
-- id: shell
-  name: cordis:group
-  group: true
-  isolate:
-    bashEnv: true
-  config:
-    # The registry and its consumer share the realm: a consumer left outside
-    # would resolve the host's `bashEnv`, which this plane no longer provides.
-    - id: bash-env
-      name: '@deepseek-ai/dsh-bash-env'
-
-    - id: tool-bash
-      name: '@deepseek-ai/dsh-tool-bash'
+# `bash-env` stays in the HOST composition: `apps/cli/src/web.ts` injects it to
+# publish `DSH_WEB_URL`/`DSH_WEB_MODE`, and a host row that injects a service is
+# the criterion for host-plane ownership — injection resolves before any session
+# exists, so there is no agent to key by. Behind a preset realm those variables
+# never reached the model's shell at all. `tool-bash` consumes the host registry
+# from here; the executor behind it (`bash-sandbox`) is host-plane too, where the
+# sandbox policy owns it.
+- id: tool-bash
+  name: '@deepseek-ai/dsh-tool-bash'
 
 # ── filesystem ──────────────────────────────────────────────────────────────
 
@@ -217,9 +210,6 @@
         toolName: subagent_fork
         backgroundMode: continuable
 
-    - id: tool-subagent-report
-      name: '@deepseek-ai/dsh-tool-subagent-report'
-
     - id: workflow-workerthread
       name: '@deepseek-ai/dsh-workflow-workerthread'
       config:

+ 70 - 0
scripts/verify-cordis-config.ts

@@ -78,6 +78,7 @@ for (const file of files) {
 errors.push(...validateExampleResolution())
 errors.push(...validateAppResolution())
 errors.push(...validateSourcePlaneResolution())
+errors.push(...validatePresetPlaneSeparation())
 
 if (errors.length > 0) {
   console.error('verify-cordis-config: invalid Loader metadata or plugin package resolution:')
@@ -87,6 +88,75 @@ if (errors.length > 0) {
   console.log(`verify-cordis-config: ${files.length} config files passed.`)
 }
 
+/**
+ * No shipped agent preset may repeat a row the host composition still runs.
+ *
+ * A preset contributes what ONE session adds to the host's registries. A row
+ * active on both planes is therefore mounted twice — once per process and once
+ * per session — and what that costs depends on what the row does: a provider
+ * behind an `isolate` realm shadows the host's for its own consumers, so a host
+ * contributor to that service reaches nobody; a row that registers into a host
+ * singleton registers once per live session, so the second one collides.
+ *
+ * Both have happened. `bash-env` in a preset realm left `DSH_WEB_URL` reaching
+ * no shell, and `tool-subagent-report` handed every child `report` once per live
+ * session until the second registration threw. Neither changes a tool catalog,
+ * so no catalog assertion can see them — and the shipped presets are near-copies
+ * of each other, so a fix applied to three of four is the normal failure.
+ * @returns one diagnostic per preset row that is also active on the host plane.
+ */
+function validatePresetPlaneSeparation(): string[] {
+  const problems: string[] = []
+  const hostFile = 'apps/cli/config/base.cordis.yml'
+  const overlayFile = 'apps/cli/config/web.cordis.yml'
+  const hostRows = rowIds(hostFile)
+  const overlay = loadEntries(overlayFile)
+  const disabled = new Set<string>()
+  for (const entry of overlay) {
+    if (!isRecord(entry)) continue
+    if (entry.disabled === true && typeof entry.id === 'string') disabled.add(entry.id)
+  }
+  // The overlay's own inserts are host-plane too; its disables take them back out.
+  const active = new Set([...hostRows, ...rowIds(overlayFile)].filter(id => !disabled.has(id)))
+  for (const file of globSync('apps/cli/config/agent-presets/*/agent.cordis.yml', { cwd: root })) {
+    for (const id of rowIds(file)) {
+      if (!active.has(id)) continue
+      problems.push(
+        `${file}: row "${id}" is also active in the host composition; `
+        + 'a row belongs to exactly one plane',
+      )
+    }
+  }
+  return problems
+}
+
+/** Every entry of one config file, or an empty list when it is not an entry array. */
+function loadEntries(file: string): unknown[] {
+  const document: unknown = yaml.load(readFileSync(resolve(root, file), 'utf8'), { schema })
+  return isUnknownArray(document) ? document : []
+}
+
+/**
+ * Row ids declared anywhere in one config file, including inside group `config`
+ * lists — a preset nests most of its rows in `isolate` groups.
+ * @param file - repository-relative config path.
+ * @returns the declared ids.
+ */
+function rowIds(file: string): Set<string> {
+  const ids = new Set<string>()
+  const walk = (value: unknown): void => {
+    if (isUnknownArray(value)) {
+      for (const item of value) walk(item)
+      return
+    }
+    if (!isRecord(value)) return
+    if (typeof value.id === 'string' && typeof value.name === 'string') ids.add(value.id)
+    for (const child of Object.values(value)) walk(child)
+  }
+  walk(loadEntries(file))
+  return ids
+}
+
 function validateEntry(value: unknown, file: string, path: string): void {
   if (!isRecord(value)) {
     errors.push(`${file}${path}: entry must be an object`)