chore(client): enforce value dependency policy
This commit is contained in:
parent
81c922c7be
commit
d80419f4ea
5 changed files with 195 additions and 17 deletions
|
|
@ -0,0 +1,6 @@
|
|||
# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-08-23-client-cross-package-value-dependencies.md
|
||||
2026-08-23-client-cross-package-value-dependencies.md: b4db6c75e245db37848d0389631441a585c82188
|
||||
2026-08-23-client-cross-package-value-dependencies.zh.md: 9894f743112157017c7d675eeac8adcb3aacff82
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
# Agent Note: Classifying Client cross-package value dependencies
|
||||
|
||||
Status: implemented
|
||||
|
||||
English | [中文](2026-08-23-client-cross-package-value-dependencies.zh.md)
|
||||
|
||||
## Problem
|
||||
|
||||
The Client package splits in [PR #2728](https://github.com/deepseek-ai/deepseek-harness/pull/2728) and [PR #2911](https://github.com/deepseek-ai/deepseek-harness/pull/2911) left 15 `dsh.client.external` requests in feature-plugin manifests. Those requests turned ordinary value imports into synchronous module-table ordering constraints, even when the consumer needed only a type, a small pure conversion, or access to an already-injected Cordis service.
|
||||
|
||||
Removing every import mechanically would create different coupling: a general utility package could become a miscellaneous business owner, a service could carry pure presentation transforms, or duplicated target behavior could be centralized only to satisfy clone detection. Client maintenance needs one repeatable classification before choosing where a cross-package reference belongs.
|
||||
|
||||
## Decision
|
||||
|
||||
Every Client cross-package reference is classified by what crosses the package boundary. A feature plugin does not import a runtime value from another feature plugin and does not declare `dsh.client.external`. The [Client shell layering decision](../architecture/2026-08-15-client-shells-and-dynamic-packages.md) continues to own bundle construction and module-table loading; this decision narrows how feature code uses those mechanisms.
|
||||
|
||||
| Case | Treatment | Reason |
|
||||
| --- | --- | --- |
|
||||
| Unused value or forwarding export | Delete it | A dependency without a caller has no owner to preserve. |
|
||||
| Shared declaration | Import it with `import type` from the declaring package | Erased imports retain one type authority without a runtime edge. |
|
||||
| Stateful, lifecycle-bound, or callable feature behavior | Expose it through an injected Cordis service | The providing plugin owns implementation and lifecycle; consumers depend on the service name and interface. |
|
||||
| Presentation contribution | Register it through the declaring slot | The owner controls placement while contributors remain independently loadable. |
|
||||
| Generic stateless helper or primitive | Put it in a narrow static utility package or `ui-primitives` | Multiple packages may synchronously share behavior only when it has no feature state, lifecycle, or domain authority. |
|
||||
| Small target-specific projection | Keep one local implementation in each target | Chat and Trajectory may intentionally interpret the same durable event independently; sharing code alone does not justify a feature dependency. |
|
||||
| Generated Remote artifact | Import it only in the API transport assembly that owns generated registration | Generated providers are transport wiring, not a feature package's callable helper API. |
|
||||
|
||||
Intentional target-local copies wrap only the duplicated implementation in `jscpd:ignore-start` / `jscpd:ignore-end`, with a comment naming the independent owners. The exclusion must not cover surrounding business logic. Generic behavior moves to a utility only when its semantics are stable outside every current caller; this cleanup places Workspace path formatting in `dsh-util-workspace-path`, byte encoding in `dsh-util-crypto`, and the shared reference glyph in `ui-primitives`.
|
||||
|
||||
`verify-client-packages` rejects every `dsh.client.external` declaration under `packages/client/*`. Outside that feature tree, each declaration must correspond to a production runtime import or re-export. The two retained requests are Session Controller → API Gateway and Workspace Controller → API Gateway; both are transport infrastructure. The Client bundle preset separately rejects workspace runtime imports that are neither module-table requests nor explicitly allowlisted static inputs.
|
||||
|
||||
Host-facing transport adapters remain outside the feature-plugin prohibition. Connection may use API Proxy's carrier implementation, and `api/remotes` may load a generated Host Remote provider. These imports assemble transport rather than sharing feature behavior.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Put every reused value on `uiConversation`.** Rejected because pure event-to-view conversions would become service calls or feature exports, forcing Chat, Trajectory, Approval, Question, Subagent, and Workspace to load an unrelated feature owner.
|
||||
|
||||
**Keep feature `dsh.client.external` declarations.** Rejected because successful loading would preserve the synchronous value dependency and merely make its ordering explicit.
|
||||
|
||||
**Move every repeated function into one utility package.** Rejected because target-specific interpretation would acquire a false shared owner. Only state-free behavior with meaning independent of its callers belongs in a static utility.
|
||||
|
||||
**Ignore all duplicate Client code.** Rejected because duplication remains useful evidence by default. An ignore is narrow and documents the deliberate independence of named targets.
|
||||
|
||||
## Consequences
|
||||
|
||||
The 15 feature-plugin external requests are absent, while shared declaration imports remain explicit and type-only. Feature loading order follows Cordis services and slots instead of synchronous feature-module imports.
|
||||
|
||||
Some short projection functions exist twice. Their owners can evolve independently, and clone detection still covers all code outside the annotated copies. Static utility packages gain a small public API and must remain state-free and browser-safe.
|
||||
|
||||
The rule is role-specific rather than a blanket ban on cross-package values. Infrastructure adapters and generated registration artifacts remain direct imports where loading or protocol assembly requires them, and `verify-client-packages` keeps those exceptions visible and live.
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
# Agent Note: Client 跨包值依赖分类
|
||||
|
||||
Status: implemented
|
||||
|
||||
[English](2026-08-23-client-cross-package-value-dependencies.md) | 中文
|
||||
|
||||
## 问题
|
||||
|
||||
[PR #2728](https://github.com/deepseek-ai/deepseek-harness/pull/2728) 与 [PR #2911](https://github.com/deepseek-ai/deepseek-harness/pull/2911) 拆分 Client 包后,功能插件 manifest 中还留有 15 条 `dsh.client.external` 请求。即使消费方只需要一个类型、一段小型纯转换或访问已经注入的 Cordis service,这些请求也会把普通值 import 变成同步模块表顺序约束。
|
||||
|
||||
机械删除所有 import 会产生别的耦合:通用工具包可能变成杂项业务 owner,service 可能承载纯展示转换,或者只为通过重复检测而把 target 行为集中到一处。维护 Client 时,需要先用同一套流程分类,再决定跨包引用应当放在哪里。
|
||||
|
||||
## 决策
|
||||
|
||||
每条 Client 跨包引用都按实际跨越包边界的内容分类。功能插件不从另一个功能插件导入运行时值,也不声明 `dsh.client.external`。[Client shell 分层决策](../architecture/2026-08-15-client-shells-and-dynamic-packages.zh.md)继续负责 bundle 构建与模块表加载;本决策进一步限定功能代码如何使用这些机制。
|
||||
|
||||
| 情形 | 处理方式 | 原因 |
|
||||
| --- | --- | --- |
|
||||
| 未使用的值或转发 export | 删除 | 没有调用方的依赖不需要保留 owner。 |
|
||||
| 共享声明 | 从声明方包使用 `import type` 导入 | 被擦除的 import 保留单一类型权威,但不产生运行时边。 |
|
||||
| 有状态、受生命周期约束或可调用的功能行为 | 通过注入的 Cordis service 暴露 | 提供插件拥有实现与生命周期;消费方只依赖 service 名称和接口。 |
|
||||
| 展示贡献 | 通过声明方 slot 注册 | owner 控制放置位置,各贡献方仍可独立加载。 |
|
||||
| 通用无状态辅助函数或基础组件 | 放入窄职责静态工具包或 `ui-primitives` | 只有不持有功能状态、生命周期或领域权威的行为才允许被多个包同步共享。 |
|
||||
| 小型 target 专属投影 | 每个 target 保留一份本地实现 | Chat 与 Trajectory 可以独立解释同一持久事件;仅仅复用代码不足以证明应建立功能依赖。 |
|
||||
| 生成的 Remote 产物 | 只在拥有生成注册的 API 传输组装层导入 | 生成的 provider 是传输接线,不是功能包的可调用辅助 API。 |
|
||||
|
||||
有意保留的 target 本地副本只用 `jscpd:ignore-start`/`jscpd:ignore-end` 包住重复实现,并在注释中点名相互独立的 owner;排除范围不得覆盖周围业务逻辑。只有语义独立于所有当前调用方时,通用行为才进入工具包;本次清理把 Workspace 路径格式化放入 `dsh-util-workspace-path`,把字节编码放入 `dsh-util-crypto`,把共享引用图标放入 `ui-primitives`。
|
||||
|
||||
`verify-client-packages` 拒绝 `packages/client/*` 下的所有 `dsh.client.external` 声明。在该功能树之外,每条声明都必须对应生产代码中的运行时 import 或 re-export。保留的两条请求是 Session Controller → API Gateway 与 Workspace Controller → API Gateway,二者都属于传输基础设施。Client bundle preset 还会拒绝既非模块表请求、也未被明确加入静态输入 allowlist 的 workspace 运行时 import。
|
||||
|
||||
面向 Host 的传输适配器不属于功能插件禁令。Connection 可以使用 API Proxy 的 carrier 实现,`api/remotes` 可以加载生成的 Host Remote provider;这些 import 用于组装传输,而不是共享功能行为。
|
||||
|
||||
## 考虑过的替代方案
|
||||
|
||||
**把所有复用值都放到 `uiConversation`。** 否决,因为纯 event→view 转换会变成 service 调用或功能 export,迫使 Chat、Trajectory、Approval、Question、Subagent 与 Workspace 加载一个无关的功能 owner。
|
||||
|
||||
**保留功能插件的 `dsh.client.external` 声明。** 否决,因为加载成功只会把同步值依赖的顺序显式化,不会消除该依赖。
|
||||
|
||||
**把每个重复函数都移入同一个工具包。** 否决,因为 target 专属解释会因此获得一个虚假的共享 owner。只有语义独立于调用方的无状态行为才属于静态工具。
|
||||
|
||||
**忽略全部 Client 重复代码。** 否决,因为重复默认仍是有用信号。每项 ignore 必须范围狭窄,并说明哪些具名 target 需要有意保持独立。
|
||||
|
||||
## 后果
|
||||
|
||||
15 条功能插件 external 请求已移除,共享声明 import 保持显式且仅类型化。功能加载顺序由 Cordis service 与 slot 决定,不再由同步功能模块 import 决定。
|
||||
|
||||
少量投影函数存在两份实现。各 owner 可以独立演进,重复检测仍覆盖注解副本以外的全部代码。静态工具包增加少量公共 API,并且必须保持无状态且可在浏览器运行。
|
||||
|
||||
这项规则按包角色区分,并非全面禁止跨包值。加载或协议组装需要的基础设施适配器与生成注册产物仍保留直接 import,`verify-client-packages` 则确保这些例外保持可见且确实仍被使用。
|
||||
|
|
@ -32,6 +32,7 @@ function declaration(
|
|||
dynamic: true,
|
||||
external: [],
|
||||
inject: [],
|
||||
runtimeSourceUses: {},
|
||||
...fields,
|
||||
}
|
||||
}
|
||||
|
|
@ -245,10 +246,39 @@ describe('dependency sections', () => {
|
|||
})
|
||||
|
||||
describe('module requests', () => {
|
||||
it('accepts a dynamic row supplier and its client subpath', () => {
|
||||
const ui = declaration('ui', { external: ['@deepseek-ai/dsh-client-slots/client'] })
|
||||
it('rejects runtime requests from one client feature package to another dynamic row', () => {
|
||||
const ui = declaration('ui', {
|
||||
external: ['@deepseek-ai/dsh-client-slots/client'],
|
||||
runtimeSourceUses: {
|
||||
'@deepseek-ai/dsh-client-slots': ['packages/client/ui/src/client/index.ts'],
|
||||
},
|
||||
})
|
||||
const slots = declaration('slots')
|
||||
expect(collectClientPackageViolations(facts([], { declarations: [ui, slots] }))).toEqual([])
|
||||
expect(collectClientPackageViolations(facts([], { declarations: [ui, slots] }))).toEqual([
|
||||
ui.manifest + ': client feature package requests runtime external '
|
||||
+ '"@deepseek-ai/dsh-client-slots/client"; import shared types only or call an injected Cordis service',
|
||||
])
|
||||
})
|
||||
|
||||
it('rejects stale externals and accepts a runtime import outside client feature packages', () => {
|
||||
const gateway = {
|
||||
...declaration('@deepseek-ai/dsh-api-gateway'), manifest: 'packages/api/gateway/package.json',
|
||||
}
|
||||
const stale = { ...declaration('@deepseek-ai/dsh-api-stale', {
|
||||
external: ['@deepseek-ai/dsh-api-gateway/client'],
|
||||
}), manifest: 'packages/api/stale/package.json' }
|
||||
const live = { ...declaration('@deepseek-ai/dsh-api-live', {
|
||||
external: ['@deepseek-ai/dsh-api-gateway/client'],
|
||||
runtimeSourceUses: {
|
||||
'@deepseek-ai/dsh-api-gateway': ['packages/api/live/src/client/index.ts'],
|
||||
},
|
||||
}), manifest: 'packages/api/live/package.json' }
|
||||
expect(collectClientPackageViolations(facts([], {
|
||||
declarations: [gateway, stale, live],
|
||||
}))).toEqual([
|
||||
stale.manifest + ': dsh.client.external "@deepseek-ai/dsh-api-gateway/client"'
|
||||
+ ' has no runtime import or re-export in production source; remove the stale declaration',
|
||||
])
|
||||
})
|
||||
|
||||
it('rejects an explicit baseline request', () => {
|
||||
|
|
@ -275,14 +305,16 @@ describe('module requests', () => {
|
|||
})
|
||||
|
||||
it('rejects synchronous module-request cycles but ignores inject cycles', () => {
|
||||
const a = declaration('a', {
|
||||
external: ['@deepseek-ai/dsh-client-b'],
|
||||
inject: ['@deepseek-ai/dsh-client-b'],
|
||||
})
|
||||
const b = declaration('b', {
|
||||
external: ['@deepseek-ai/dsh-client-a'],
|
||||
inject: ['@deepseek-ai/dsh-client-a'],
|
||||
})
|
||||
const a = { ...declaration('@deepseek-ai/dsh-api-a', {
|
||||
external: ['@deepseek-ai/dsh-api-b'],
|
||||
inject: ['@deepseek-ai/dsh-api-b'],
|
||||
runtimeSourceUses: { '@deepseek-ai/dsh-api-b': ['packages/api/a/src/client.ts'] },
|
||||
}), manifest: 'packages/api/a/package.json' }
|
||||
const b = { ...declaration('@deepseek-ai/dsh-api-b', {
|
||||
external: ['@deepseek-ai/dsh-api-a'],
|
||||
inject: ['@deepseek-ai/dsh-api-a'],
|
||||
runtimeSourceUses: { '@deepseek-ai/dsh-api-a': ['packages/client/b/src/client.ts'] },
|
||||
}), manifest: 'packages/api/b/package.json' }
|
||||
const found = collectClientPackageViolations(facts([], { declarations: [a, b] }))
|
||||
expect(found).toHaveLength(1)
|
||||
expect(found[0]).toContain('synchronous dsh.client.external cycle')
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ export interface ClientDeclaration {
|
|||
readonly manifest: string
|
||||
readonly dynamic: boolean
|
||||
readonly external: readonly string[]
|
||||
readonly runtimeSourceUses: Readonly<Record<string, readonly string[]>>
|
||||
/** Informational package dependencies declared by the row. */
|
||||
readonly inject: readonly string[]
|
||||
}
|
||||
|
|
@ -34,7 +35,6 @@ export interface ClientDeclaration {
|
|||
export interface ClientPackage extends ClientDeclaration {
|
||||
readonly staticLinked: boolean
|
||||
readonly sourceUses: Readonly<Record<string, readonly string[]>>
|
||||
readonly runtimeSourceUses: Readonly<Record<string, readonly string[]>>
|
||||
readonly dependencies: Readonly<Record<string, string>>
|
||||
readonly peerDependencies: Readonly<Record<string, string>>
|
||||
readonly devDependencies: Readonly<Record<string, string>>
|
||||
|
|
@ -564,6 +564,21 @@ function collectModuleViolations(facts: ClientPackageFacts): string[] {
|
|||
if (supplier === pkg.name) {
|
||||
violations.push(pkg.manifest + ': dsh.client.external names its own row ' + JSON.stringify(specifier))
|
||||
} else if (supplier !== undefined) {
|
||||
if (pkg.manifest.startsWith('packages/client/')) {
|
||||
violations.push(
|
||||
pkg.manifest + ': client feature package requests runtime external ' + JSON.stringify(specifier)
|
||||
+ '; import shared types only or call an injected Cordis service',
|
||||
)
|
||||
continue
|
||||
}
|
||||
const owner = packageNameOf(specifier)
|
||||
if (pkg.runtimeSourceUses[owner] === undefined) {
|
||||
violations.push(
|
||||
pkg.manifest + ': dsh.client.external ' + JSON.stringify(specifier)
|
||||
+ ' has no runtime import or re-export in production source; remove the stale declaration',
|
||||
)
|
||||
continue
|
||||
}
|
||||
edges.push({ from: pkg.name, to: supplier, specifier })
|
||||
} else {
|
||||
const owner = stripClientSuffix(specifier)
|
||||
|
|
@ -654,11 +669,15 @@ function readDeclaration(
|
|||
const dsh = isRecord(manifest.dsh) ? manifest.dsh : undefined
|
||||
const rawClient = dsh?.client
|
||||
if (rawClient === undefined) {
|
||||
return { name: manifest.name, manifest: manifestPath, dynamic: false, external: [], inject: [] }
|
||||
return {
|
||||
name: manifest.name, manifest: manifestPath, dynamic: false, external: [], inject: [], runtimeSourceUses: {},
|
||||
}
|
||||
}
|
||||
if (!isRecord(rawClient)) {
|
||||
malformed.push(manifestPath + ': ' + manifest.name + ' dsh.client must be an object')
|
||||
return { name: manifest.name, manifest: manifestPath, dynamic: false, external: [], inject: [] }
|
||||
return {
|
||||
name: manifest.name, manifest: manifestPath, dynamic: false, external: [], inject: [], runtimeSourceUses: {},
|
||||
}
|
||||
}
|
||||
return {
|
||||
name: manifest.name,
|
||||
|
|
@ -666,6 +685,7 @@ function readDeclaration(
|
|||
dynamic: true,
|
||||
external: stringArray(rawClient.external, manifest.name, manifestPath, 'external', malformed),
|
||||
inject: stringArray(rawClient.inject, manifest.name, manifestPath, 'inject', malformed),
|
||||
runtimeSourceUses: {},
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -747,10 +767,32 @@ function readStringLiteralArray(root: string, sourcePath: string, name: string):
|
|||
}
|
||||
|
||||
async function readFacts(root: string): Promise<ClientPackageFacts> {
|
||||
const { declarations, malformed } = readClientDeclarations(root)
|
||||
const byManifest = new Map(declarations.map(entry => [entry.manifest, entry]))
|
||||
const { declarations: bareDeclarations, malformed } = readClientDeclarations(root)
|
||||
const staticLinkedPackages = await readStaticLinkedRoster(root)
|
||||
const project = new TypeScriptProject(root, 'client')
|
||||
const sourceFiles = project.sourceFiles()
|
||||
const declarations = bareDeclarations.map((declaration): ClientDeclaration => {
|
||||
const runtimeSourceUses = new Map<string, Set<string>>()
|
||||
const sourcePrefix = dirname(declaration.manifest) + '/src/'
|
||||
for (const sourceFile of sourceFiles) {
|
||||
if (sourceFile.isDeclarationFile) continue
|
||||
const file = project.relativePath(sourceFile)
|
||||
if (!file.startsWith(sourcePrefix)) continue
|
||||
for (const name of collectSourceFilePackageUses(sourceFile, true)) {
|
||||
const locations = runtimeSourceUses.get(name) ?? new Set<string>()
|
||||
locations.add(file)
|
||||
runtimeSourceUses.set(name, locations)
|
||||
}
|
||||
}
|
||||
return {
|
||||
...declaration,
|
||||
runtimeSourceUses: Object.fromEntries(
|
||||
[...runtimeSourceUses].sort(([left], [right]) => left.localeCompare(right))
|
||||
.map(([name, locations]) => [name, [...locations].sort()]),
|
||||
),
|
||||
}
|
||||
})
|
||||
const byManifest = new Map(declarations.map(entry => [entry.manifest, entry]))
|
||||
const packages: ClientPackage[] = []
|
||||
|
||||
for (const manifestPath of globSync(CLIENT_MANIFEST_GLOB, { cwd: root }).map(normalizePath).sort()) {
|
||||
|
|
@ -762,7 +804,7 @@ async function readFacts(root: string): Promise<ClientPackageFacts> {
|
|||
const runtimeSourceUses = new Map<string, Set<string>>()
|
||||
const packageDirectory = dirname(manifestPath)
|
||||
const sourcePrefix = packageDirectory + '/src/'
|
||||
for (const sourceFile of project.sourceFiles()) {
|
||||
for (const sourceFile of sourceFiles) {
|
||||
if (sourceFile.isDeclarationFile) continue
|
||||
const file = project.relativePath(sourceFile)
|
||||
if (!file.startsWith(sourcePrefix)) continue
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue