fix review findings: stale coding-agent README, export-shape guards, overclaim wording

Codex review of PR #88 found three issues in the example-app extraction:

A1 — examples/coding-agent/README.md's plugin table still listed the OLD
direct-wired leaf entries (agent-loop, session-persistence, src/stdio-chat.ts —
the whole src/ dir is gone). Rewrite it to the four real leaf entries the
current cordis.yml loads (hmr, llm-deepseek, bash, stdio-agent), noting that
tool-bash/persistence/agent/loop now live inside the agent-core + stdio-agent
bundles.

A2 — the three new app/spine packages (agent-core, stdio-agent, acp-agent)
export NO `inject`, so a stray `export default apply` would let unwrapExports
collapse the module and silently DROP name/Config WITHOUT crashing — the
real-load-path smokes would stay green. agent-core is never Loader-unwrapped at
all. Add an explicit export-shape guard per package: assert no `default` export
and that the real Loader.unwrapExports leaves name/Config/apply intact. Verified
each fails when `export default apply` is added.

B — soften "structurally unreachable / cannot wire a stdout logger" overclaims
in the acp-agent/agent-core READMEs and the implemented RFC: a leaf CAN still add
a sibling logger entry; the accurate claim is the app omits one so the default
leaf has nothing to get wrong. Keep the safety directive (never add a stdout
logger to an ACP leaf).
This commit is contained in:
Tianyi Cui
2026-06-21 12:49:59 +08:00
parent 47afcb297f
commit 66e56bd395
8 changed files with 76 additions and 11 deletions

View File

@@ -1,5 +1,6 @@
import { describe, it, expect } from 'vitest'
import { Context } from 'cordis'
import Loader from '@cordisjs/plugin-loader'
import { AgentId } from '@deepseek-ai/dsh-agent'
import * as stdioAgent from '../src/index.ts'
@@ -68,4 +69,24 @@ describe('dsh-stdio-agent app', () => {
expect(stdioAgent.name).toBe('stdio-agent')
expect(stdioAgent.Config).toBeDefined()
})
it('has the namespace-plugin export shape (no stray default) so the Loader keeps name/Config/apply', () => {
// Postmortem 0001 guard: a stray `export default apply` makes the Loader's
// `unwrapExports` (`exports.default ?? exports`) collapse the module to the
// bare `apply` function, DROPPING the named `name`/`Config`. This package has
// no `inject` export, so that collapse would NOT crash at load (the keyless
// echo smoke would still boot the tree) — it would silently lose its config
// schema. So guard the shape directly here: assert no `default` export, and
// that the real `unwrapExports` leaves `name`/`Config`/`apply` intact. Adding
// `export default` to src/index.ts fails this test.
expect('default' in stdioAgent).toBe(false)
expect(typeof stdioAgent.apply).toBe('function')
const loader = Object.create(Loader.prototype) as Loader
const unwrapped = loader.unwrapExports(stdioAgent) as Record<string, unknown>
expect(unwrapped).toBe(stdioAgent)
expect(unwrapped.name).toBe('stdio-agent')
expect(unwrapped.Config).toBeDefined()
expect(typeof unwrapped.apply).toBe('function')
})
})