From 47399764c5e245f1066a68f87bd5a65206d75d7f Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:27:13 +0800 Subject: [PATCH 1/6] fix(release): order publication by every installed dependency section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Publish order exists to make a partial publication self-consistent: an interrupted run should leave a prefix whose packages never point at a version absent from the registry. It read only dependencies and optionalDependencies, so peer declarations — how sibling harness packages reference each other, 1088 edges in the dsh family — constrained nothing. Peer edges now order the publication too. devDependencies still do not: a dev dependency is absent from the published package. Peers cannot constrain it absolutely. Sibling packages declare each other as peers, which is what closes the two cycles here, and npm treats an unmet peer as a warning rather than a resolution failure. Install edges therefore win: a peer edge is dropped where the peer installs the member declaring it, or where following it would revisit a member already being visited. One peer edge is dropped in the dsh family and two in the vendored family; every install edge is honoured. A cycle among install edges stays a defect rather than something to order around, and release:verify now reports it before the build instead of letting it surface once pack is already writing tarballs. Install-edge acyclicity is checked on its own graph, because a peer edge leading into an install edge otherwise reads as a cycle where the install edges are perfectly orderable. --- scripts/release/families.spec.ts | 62 +++++++++++++++++++ scripts/release/families.ts | 103 ++++++++++++++++++++++++------- scripts/release/verify.ts | 11 +++- 3 files changed, 154 insertions(+), 22 deletions(-) diff --git a/scripts/release/families.spec.ts b/scripts/release/families.spec.ts index 369fc62b16..2c872d4487 100644 --- a/scripts/release/families.spec.ts +++ b/scripts/release/families.spec.ts @@ -74,6 +74,68 @@ describe('release families', () => { expect(() => { dsh.publishOrder(members) }).toThrow(/dependency cycle/) }) + it('publishes a peer before its consumer', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/consumer', '@deepseek-ai/dsh-consumer', { peerDependencies: { '@deepseek-ai/dsh-zebra': 'workspace:^' } }), + member('packages/a/zebra', '@deepseek-ai/dsh-zebra'), + ] + + // Name order alone would place the consumer first; the peer edge moves it. + expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-zebra', + '@deepseek-ai/dsh-consumer', + ]) + }) + + it('orders around a peer cycle rather than refusing to publish', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/left', '@deepseek-ai/dsh-left', { peerDependencies: { '@deepseek-ai/dsh-right': 'workspace:^' } }), + member('packages/a/right', '@deepseek-ai/dsh-right', { peerDependencies: { '@deepseek-ai/dsh-left': 'workspace:^' } }), + ] + + // Sibling packages declare each other as peers, and npm treats an unmet peer + // as a warning, so this pair has to publish rather than fail the release. + expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-right', + '@deepseek-ai/dsh-left', + ]) + }) + + it('honours an install edge even when a peer cycle surrounds it', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/base', '@deepseek-ai/dsh-base', { peerDependencies: { '@deepseek-ai/dsh-consumer': 'workspace:^' } }), + member('packages/a/consumer', '@deepseek-ai/dsh-consumer', { + dependencies: { '@deepseek-ai/dsh-base': 'workspace:^' }, + peerDependencies: { '@deepseek-ai/dsh-base': 'workspace:^' }, + }), + ] + + // The install edge is absolute: base publishes first, and the peer edge that + // would reverse it is the one dropped. + expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-base', + '@deepseek-ai/dsh-consumer', + ]) + }) + + it('ignores devDependencies when ordering', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/alpha', '@deepseek-ai/dsh-alpha', { devDependencies: { '@deepseek-ai/dsh-zebra': 'workspace:^' } }), + member('packages/a/zebra', '@deepseek-ai/dsh-zebra'), + ] + + // A dev dependency is absent from the published package, so it must not move + // the consumer behind it. + expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-alpha', + '@deepseek-ai/dsh-zebra', + ]) + }) + it('applies the harness payload policy to dsh and keeps upstream payloads for vendored packages', () => { const dsh = releaseFamily('dsh') const vendor = releaseFamily('vendor') diff --git a/scripts/release/families.ts b/scripts/release/families.ts index d39552636d..2135920047 100644 --- a/scripts/release/families.ts +++ b/scripts/release/families.ts @@ -13,8 +13,21 @@ import { globSync, readFileSync } from 'node:fs' import { resolve } from 'node:path' import { validateTarballPayload } from '../publication-payload.ts' -/** Dependency sections that constrain publish order: a consumer must publish after its dependency. */ -const ORDER_SECTIONS = ['dependencies', 'optionalDependencies'] as const +/** + * Dependency sections a consumer must publish after, because npm resolves them + * when the package is installed: publishing a consumer first would leave a + * window where its own tree cannot be assembled. + */ +const INSTALL_SECTIONS = ['dependencies', 'optionalDependencies'] as const + +/** + * Peer declarations also order the publication, but they cannot constrain it. + * npm never installs a peer on the package's behalf — an unmet peer is a + * warning, not a resolution failure — and sibling packages legitimately declare + * each other as peers, which makes these edges the ones that close cycles. They + * order what they can and are dropped where they would deadlock. + */ +const PEER_SECTIONS = ['peerDependencies'] as const /** The workspace root manifest, which is never a release member. */ const WORKSPACE_ROOT_PACKAGE = '@deepseek-ai/dsh-root' @@ -107,45 +120,93 @@ export abstract class ReleaseFamily { } /** - * Order members so every package publishes after the family members it depends on. + * Order members so every package publishes after the family members it + * depends on, which is what makes a partial publication self-consistent: an + * interrupted run leaves a prefix whose packages never point at something + * absent from the registry. + * + * Install edges are honoured absolutely — a cycle among them is a defect this + * reports rather than works around. Peer edges order what they can and are + * dropped where honouring one would deadlock: sibling packages declare each + * other as peers, and npm treats an unmet peer as a warning rather than a + * resolution failure ([rationale](../../.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md)). * @param members - this family's members. * @returns The same members in publish order; ties break by name for determinism. */ publishOrder(members: readonly ReleaseMember[]): ReleaseMember[] { const byName = new Map(members.map(member => [member.name, member])) - const ordered: ReleaseMember[] = [] - const placed = new Set() - const visiting = new Set() + const byNameSorted = [...members].sort((left, right) => left.name.localeCompare(right.name)) + const edges = (member: ReleaseMember, sections: readonly string[]): ReleaseMember[] => + this.orderEdges(member, byName, sections) - const visit = (member: ReleaseMember, path: readonly string[]): void => { - if (placed.has(member.name)) return - if (visiting.has(member.name)) { + // Install edges alone must be acyclic, and that is checked on its own graph: + // a peer edge leading into an install edge would otherwise read as a cycle + // where the install edges are perfectly orderable. + const installVisiting = new Set() + const installDone = new Set() + const checkInstall = (member: ReleaseMember, path: readonly string[]): void => { + if (installDone.has(member.name)) return + if (installVisiting.has(member.name)) { throw new Error(`dependency cycle in release family ${this.id}: ${[...path, member.name].join(' -> ')}`) } - visiting.add(member.name) - for (const dependency of this.orderEdges(member, byName)) { - visit(dependency, [...path, member.name]) + installVisiting.add(member.name) + for (const dependency of edges(member, INSTALL_SECTIONS)) checkInstall(dependency, [...path, member.name]) + installVisiting.delete(member.name) + installDone.add(member.name) + } + for (const member of byNameSorted) checkInstall(member, []) + + // Emit the order over both kinds of edge. A node already on the stack is a + // cycle only peer edges can form, and skipping it drops just that edge. + const ordered: ReleaseMember[] = [] + const placed = new Set() + const onStack = new Set() + // Members reachable from one member through install edges. A peer edge is + // dropped when the peer installs the member declaring it: honouring it would + // emit a package before something it installs, and the install edge wins. + const installClosure = (member: ReleaseMember): Set => { + const reached = new Set() + const walk = (current: ReleaseMember): void => { + for (const dependency of edges(current, INSTALL_SECTIONS)) { + if (reached.has(dependency.name)) continue + reached.add(dependency.name) + walk(dependency) + } } - visiting.delete(member.name) + walk(member) + return reached + } + const visit = (member: ReleaseMember): void => { + if (placed.has(member.name) || onStack.has(member.name)) return + onStack.add(member.name) + for (const dependency of edges(member, INSTALL_SECTIONS)) visit(dependency) + for (const peer of edges(member, PEER_SECTIONS)) { + if (installClosure(peer).has(member.name)) continue + visit(peer) + } + onStack.delete(member.name) + if (placed.has(member.name)) return placed.add(member.name) ordered.push(member) } - - for (const member of [...members].sort((left, right) => left.name.localeCompare(right.name))) { - visit(member, []) - } + for (const member of byNameSorted) visit(member) return ordered } /** - * The family members one member depends on at runtime. + * The family members one member declares in the given sections. * @param member - the dependent member. * @param byName - every family member by package name. - * @returns Dependencies inside this family, sorted by name. + * @param sections - manifest sections to read. + * @returns Members of this family named there, sorted by name. */ - private orderEdges(member: ReleaseMember, byName: ReadonlyMap): ReleaseMember[] { + private orderEdges( + member: ReleaseMember, + byName: ReadonlyMap, + sections: readonly string[], + ): ReleaseMember[] { const edges: ReleaseMember[] = [] - for (const section of ORDER_SECTIONS) { + for (const section of sections) { const dependencies = member.manifest[section] if (dependencies === null || typeof dependencies !== 'object' || Array.isArray(dependencies)) continue for (const name of Object.keys(dependencies)) { diff --git a/scripts/release/verify.ts b/scripts/release/verify.ts index 1bd74c84d6..bd906bcc9e 100644 --- a/scripts/release/verify.ts +++ b/scripts/release/verify.ts @@ -55,6 +55,15 @@ function main(): void { const family = releaseFamily(values.family) const members = family.members(process.cwd()) family.verifyVersions(members) + // Resolve the publish order here, before the build: an install-edge cycle + // makes the order unrepresentable, and that has to surface at the first gate + // rather than when pack is already writing tarballs. + const ordered = family.publishOrder(members) + if (ordered.length !== members.length) { + throw new Error( + `release family ${family.id}: publish order covers ${String(ordered.length)} of ${String(members.length)} members`, + ) + } const publishing = process.env.RELEASE_PUBLISH === 'true' if (publishing) { @@ -64,7 +73,7 @@ function main(): void { const versions = [...new Set(members.map(member => member.version))] const summary = versions.length === 1 ? versions[0] : `${String(versions.length)} versions` - console.log(`release verify: family ${family.id}, ${String(members.length)} member(s), ${summary}${publishing ? ', publish gates passed' : ''}`) + console.log(`release verify: family ${family.id}, ${String(members.length)} member(s), ${summary}, publish order resolved${publishing ? ', publish gates passed' : ''}`) } if (isEntry(import.meta.url)) main() From 70eb76eaecb73b3cd953ce9b4fcbbf78d1f6cc6d Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:27:14 +0800 Subject: [PATCH 2/6] fix(release): keep npm's own output in the publish log Retry classification needs npm's failure text, so the publish call captured its streams instead of inheriting them. That silenced npm on the success path: the log lost the tarball contents, the notices, and the '+ name@version' confirmation for every package. Pipe the streams and echo them, so the log shows what npm reported and the caller still gets the text it classifies. The registry probe behind it keeps its streams captured, since its JSON and its E404 are internal queries rather than progress. --- scripts/release/process.ts | 26 ++++++++++++++++++++++++++ scripts/release/publish.ts | 4 ++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/scripts/release/process.ts b/scripts/release/process.ts index 746f24ac36..acec98feae 100644 --- a/scripts/release/process.ts +++ b/scripts/release/process.ts @@ -38,6 +38,32 @@ export function attempt(command: string, args: readonly string[], options: RunOp return { status: result.status, stdout: result.stdout, stderr: result.stderr } } +/** + * Run a command, letting its output reach the log while also returning it. + * + * A step that both shows progress and classifies its own failure needs both: the + * output has to appear in the workflow log as the command produces it, and the + * caller has to read it to decide whether a failure is worth retrying. + * @param command - executable name. + * @param args - command arguments. + * @param options - working directory and environment. + * @returns The exit status and captured streams. + */ +export function attemptStreaming(command: string, args: readonly string[], options: RunOptions = {}): CommandResult { + const result = spawnSync(command, [...args], { + cwd: options.cwd, + env: options.env, + encoding: 'utf8', + // 'inherit' would leave nothing to capture, so the streams are piped and + // echoed instead. + stdio: ['inherit', 'pipe', 'pipe'], + }) + if (result.error !== undefined) throw result.error + if (result.stdout !== '') process.stdout.write(result.stdout) + if (result.stderr !== '') process.stderr.write(result.stderr) + return { status: result.status, stdout: result.stdout, stderr: result.stderr } +} + /** * Run a command, capture its standard output, and fail on a non-zero exit. * @param command - executable name. diff --git a/scripts/release/publish.ts b/scripts/release/publish.ts index 11ad01173c..f861da18c2 100644 --- a/scripts/release/publish.ts +++ b/scripts/release/publish.ts @@ -18,7 +18,7 @@ import { join, resolve } from 'node:path' import { setTimeout as sleep } from 'node:timers/promises' import { parseArgs } from 'node:util' import { releaseFamily } from './families.ts' -import { attempt, isEntry } from './process.ts' +import { attempt, attemptStreaming, isEntry } from './process.ts' import { packedIdentity, readPublishOrder } from './tarball.ts' /** @@ -102,7 +102,7 @@ async function publishTarball(tarball: string, name: string, version: string): P // command-line flag could not serve both and would override the manifest // that does. Each packed manifest decides, and // check-workspace-constraints holds every manifest to its sequence's level. - const result = attempt('npm', ['publish', tarball, ...tagArgs]) + const result = attemptStreaming('npm', ['publish', tarball, ...tagArgs]) const output = `${result.stdout}${result.stderr}` if (result.status === 0) return From 9fa0575ccc05970974a4cdbb29b096fc70848cba Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:38:35 +0800 Subject: [PATCH 3/6] fix(release): print the publish order and the peer edges it drops The verify step resolved the publish order and said only that it had: the order a release actually follows, and the ordering it could not honour, stayed invisible until a publication was already running. publishOrder now returns that order together with the peer edges it dropped, verify prints both, and pack reads the order off the plan. The dropped edges are part of the result rather than a detail of forming it: the dsh family drops one (dsh-api-remotes -> dsh-api-gateway) and the vendored family drops two (cordis-plugin-include and cordis-plugin-loader, which cordis declares as peers in return), and only whoever reads the log can judge whether a newly dropped edge is expected. Because pack runs on every pull request and master push, a change to the order is now reviewable there rather than observable only at publish time. The order is also checked against the edges it exists to honour. A cycle mixing peer and dependency declarations can put a dependency on the traversal stack, where it is skipped like a peer edge, emitting a consumer before something it installs; no later step can detect that, and it would surface as an unresolvable install for a consumer of the published packages. No family has that shape today, and the new test pins the three-package case that would. --- ...2026-08-10-npm-release-sequences.i18n.yaml | 4 +- .../2026-08-10-npm-release-sequences.md | 4 +- .../2026-08-10-npm-release-sequences.zh.md | 4 +- scripts/release/families.spec.ts | 36 ++++++++++-- scripts/release/families.ts | 57 +++++++++++++++++-- scripts/release/pack.ts | 2 +- scripts/release/verify.ts | 41 +++++++++++-- 7 files changed, 126 insertions(+), 22 deletions(-) diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml index 59b51bbe6b..4b14d3b8aa 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml @@ -2,5 +2,5 @@ # 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-10-npm-release-sequences.md -2026-08-10-npm-release-sequences.md: e74a4ac8f2aadd8665ec0db198c6a317a0c201bc -2026-08-10-npm-release-sequences.zh.md: e152163976f945224f2524fccd7f831ba98e8161 +2026-08-10-npm-release-sequences.md: 2c46fb9b3e3fb8ddd90131e3c3113166580608d4 +2026-08-10-npm-release-sequences.zh.md: edbb2a8884f658b6c87ceb5762c01551a81a249d diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md index e74a4ac8f2..2c46fb9b3e 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md @@ -88,9 +88,9 @@ The entity in this domain is a **release family**: a set of packages sharing one |---|---| | `ReleaseFamily` | a family's identity: member discovery, version baseline, tag prefix, packed-payload rule, installed entry | | `ReleaseMember` | one publishable package: directory, name, version, manifest | -| `publishOrder` | topological order over runtime dependencies, ties broken by package name; a cycle is reported rather than resolved arbitrarily | +| `publishOrder` | topological order over the sections npm installs plus peer declarations, ties broken by package name; a cycle among installed dependencies is reported rather than resolved arbitrarily, and a peer edge no order can honour is dropped and named | | `pack` | packs a whole family into one directory and records the upload order | -| `verify` | the family's version baseline, and — when publishing — that the run comes from that family's tag and its members are publishable | +| `verify` | the family's version baseline, the publish order it prints in full, and — when publishing — that the run comes from that family's tag and its members are publishable | | `verify-packed-install` | installs the tarballs of one or more pack directories into a throwaway consumer and drives the installed executable | | `publish` | the three registry states above | | `process` / `tarball` | the one home for spawning commands and for reading a packed tarball, including the entry guard that keeps every script importable | diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md index e152163976..edbb2a8884 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md @@ -88,9 +88,9 @@ registry 的两个行为决定了「怎么尝试一次发布」。写入之间 |---|---| | `ReleaseFamily` | 一族的身份:成员发现、版本基线、tag 前缀、打包 payload 规则、已安装入口 | | `ReleaseMember` | 一个可发布包:目录、包名、版本、manifest | -| `publishOrder` | 按运行时依赖的拓扑序,同层按包名排;遇到环是报错而不是随意定序 | +| `publishOrder` | 按 npm 会安装的依赖段加 peer 声明做拓扑序,同层按包名排;安装依赖成环是报错而不是随意定序,任何排不进去的 peer 边被丢弃并点名 | | `pack` | 把整族打进一个目录并记录上传顺序 | -| `verify` | 族的版本基线;发布时还要求本次运行来自该族的 tag、且成员可发布 | +| `verify` | 族的版本基线、完整打印出来的发布顺序;发布时还要求本次运行来自该族的 tag、且成员可发布 | | `verify-packed-install` | 把一个或多个 pack 目录的 tarball 装进一次性 consumer,并驱动已安装的可执行入口 | | `publish` | 上面那三态 | | `process` / `tarball` | 启动命令、读取打包 tarball 的唯一正家,其中的入口守卫让每个脚本都可被 import | diff --git a/scripts/release/families.spec.ts b/scripts/release/families.spec.ts index 2c872d4487..66c3daf83f 100644 --- a/scripts/release/families.spec.ts +++ b/scripts/release/families.spec.ts @@ -57,7 +57,7 @@ describe('release families', () => { member('packages/a/zebra', '@deepseek-ai/dsh-zebra'), ] - expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + expect(dsh.publishOrder(members).order.map(entry => entry.name)).toEqual([ '@deepseek-ai/dsh-library', '@deepseek-ai/dsh-consumer', '@deepseek-ai/dsh-zebra', @@ -82,13 +82,13 @@ describe('release families', () => { ] // Name order alone would place the consumer first; the peer edge moves it. - expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + expect(dsh.publishOrder(members).order.map(entry => entry.name)).toEqual([ '@deepseek-ai/dsh-zebra', '@deepseek-ai/dsh-consumer', ]) }) - it('orders around a peer cycle rather than refusing to publish', () => { + it('orders around a peer cycle rather than refusing to publish, and reports the edge it dropped', () => { const dsh = releaseFamily('dsh') const members = [ member('packages/a/left', '@deepseek-ai/dsh-left', { peerDependencies: { '@deepseek-ai/dsh-right': 'workspace:^' } }), @@ -97,10 +97,15 @@ describe('release families', () => { // Sibling packages declare each other as peers, and npm treats an unmet peer // as a warning, so this pair has to publish rather than fail the release. - expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + const plan = dsh.publishOrder(members) + expect(plan.order.map(entry => entry.name)).toEqual([ '@deepseek-ai/dsh-right', '@deepseek-ai/dsh-left', ]) + // One of the two edges has to give, and which one it is belongs in the log. + expect(plan.droppedPeerEdges).toEqual([ + { consumer: '@deepseek-ai/dsh-right', peer: '@deepseek-ai/dsh-left' }, + ]) }) it('honours an install edge even when a peer cycle surrounds it', () => { @@ -115,10 +120,29 @@ describe('release families', () => { // The install edge is absolute: base publishes first, and the peer edge that // would reverse it is the one dropped. - expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + const plan = dsh.publishOrder(members) + expect(plan.order.map(entry => entry.name)).toEqual([ '@deepseek-ai/dsh-base', '@deepseek-ai/dsh-consumer', ]) + expect(plan.droppedPeerEdges).toEqual([ + { consumer: '@deepseek-ai/dsh-base', peer: '@deepseek-ai/dsh-consumer' }, + ]) + }) + + it('refuses an order that would publish a consumer before a dependency it installs', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/alpha', '@deepseek-ai/dsh-alpha', { peerDependencies: { '@deepseek-ai/dsh-bravo': 'workspace:^' } }), + member('packages/a/bravo', '@deepseek-ai/dsh-bravo', { peerDependencies: { '@deepseek-ai/dsh-charlie': 'workspace:^' } }), + member('packages/a/charlie', '@deepseek-ai/dsh-charlie', { dependencies: { '@deepseek-ai/dsh-alpha': 'workspace:^' } }), + ] + + // A cycle of two peer edges closed by one install edge: dropping a peer edge + // would order this, and the traversal drops the install edge instead. That + // order would publish charlie before the alpha it installs, so it is refused + // here rather than published. + expect(() => { dsh.publishOrder(members) }).toThrow(/no publish order honours @deepseek-ai\/dsh-charlie -> @deepseek-ai\/dsh-alpha/) }) it('ignores devDependencies when ordering', () => { @@ -130,7 +154,7 @@ describe('release families', () => { // A dev dependency is absent from the published package, so it must not move // the consumer behind it. - expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + expect(dsh.publishOrder(members).order.map(entry => entry.name)).toEqual([ '@deepseek-ai/dsh-alpha', '@deepseek-ai/dsh-zebra', ]) diff --git a/scripts/release/families.ts b/scripts/release/families.ts index 2135920047..09acbe2da9 100644 --- a/scripts/release/families.ts +++ b/scripts/release/families.ts @@ -32,6 +32,28 @@ const PEER_SECTIONS = ['peerDependencies'] as const /** The workspace root manifest, which is never a release member. */ const WORKSPACE_ROOT_PACKAGE = '@deepseek-ai/dsh-root' +/** One peer declaration the publish order leaves unordered. */ +interface DroppedPeerEdge { + /** Package declaring the peer. */ + readonly consumer: string + /** The declared peer, which publishes after `consumer` or alongside it in a cycle. */ + readonly peer: string +} + +/** + * A family's publish order together with the ordering it could not honour. + * + * The dropped edges are part of the result rather than a detail of forming it: + * a release drops real ordering constraints, and the operator reading the pack + * log is the only one who can judge whether a newly dropped edge is expected. + */ +export interface PublishPlan { + /** Members in publish order. */ + readonly order: readonly ReleaseMember[] + /** Peer declarations left unordered, in the order the traversal reached them. */ + readonly droppedPeerEdges: readonly DroppedPeerEdge[] +} + /** One publishable package of a release family. */ export interface ReleaseMember { /** Repository-relative package directory, for example `packages/core/session`. */ @@ -130,10 +152,12 @@ export abstract class ReleaseFamily { * dropped where honouring one would deadlock: sibling packages declare each * other as peers, and npm treats an unmet peer as a warning rather than a * resolution failure ([rationale](../../.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md)). + * Every dropped edge is reported, because dropping one is a decision about a + * real release rather than an implementation detail. * @param members - this family's members. - * @returns The same members in publish order; ties break by name for determinism. + * @returns The order, ties broken by name for determinism, and the peer edges it left unordered. */ - publishOrder(members: readonly ReleaseMember[]): ReleaseMember[] { + publishOrder(members: readonly ReleaseMember[]): PublishPlan { const byName = new Map(members.map(member => [member.name, member])) const byNameSorted = [...members].sort((left, right) => left.name.localeCompare(right.name)) const edges = (member: ReleaseMember, sections: readonly string[]): ReleaseMember[] => @@ -159,6 +183,7 @@ export abstract class ReleaseFamily { // Emit the order over both kinds of edge. A node already on the stack is a // cycle only peer edges can form, and skipping it drops just that edge. const ordered: ReleaseMember[] = [] + const droppedPeerEdges: DroppedPeerEdge[] = [] const placed = new Set() const onStack = new Set() // Members reachable from one member through install edges. A peer edge is @@ -181,7 +206,13 @@ export abstract class ReleaseFamily { onStack.add(member.name) for (const dependency of edges(member, INSTALL_SECTIONS)) visit(dependency) for (const peer of edges(member, PEER_SECTIONS)) { - if (installClosure(peer).has(member.name)) continue + if (installClosure(peer).has(member.name)) { + droppedPeerEdges.push({ consumer: member.name, peer: peer.name }) + continue + } + // A peer already on the stack is an ancestor, so it publishes after this + // member rather than before it: the edge is dropped, not honoured. + if (onStack.has(peer.name)) droppedPeerEdges.push({ consumer: member.name, peer: peer.name }) visit(peer) } onStack.delete(member.name) @@ -190,7 +221,25 @@ export abstract class ReleaseFamily { ordered.push(member) } for (const member of byNameSorted) visit(member) - return ordered + + // A cycle mixing both kinds of edge can put an install edge's target on the + // stack, where the traversal skips it like a peer edge and emits a consumer + // before something it installs. Nothing downstream can detect that, and it + // would only surface as an unresolvable install for whoever consumes the + // published packages, so the emitted order is checked against the edges it + // exists to honour. + const position = new Map(ordered.map((entry, index) => [entry.name, index])) + for (const [index, member] of ordered.entries()) { + for (const dependency of edges(member, INSTALL_SECTIONS)) { + const dependencyIndex = position.get(dependency.name) + if (dependencyIndex !== undefined && dependencyIndex < index) continue + throw new Error( + `release family ${this.id}: no publish order honours ${member.name} -> ${dependency.name};` + + ' a cycle mixing peer and dependency declarations reaches this dependency through a peer edge', + ) + } + } + return { order: ordered, droppedPeerEdges } } /** diff --git a/scripts/release/pack.ts b/scripts/release/pack.ts index 47a33a26ac..5d2b9b4e64 100644 --- a/scripts/release/pack.ts +++ b/scripts/release/pack.ts @@ -45,7 +45,7 @@ function main(): void { const family = releaseFamily(values.family) const root = process.cwd() const destination = resolve(root, values.out ?? DEFAULT_OUTPUT) - const members = family.publishOrder(family.members(root)) + const members = family.publishOrder(family.members(root)).order family.verifyVersions(members) rmSync(destination, { recursive: true, force: true }) diff --git a/scripts/release/verify.ts b/scripts/release/verify.ts index bd906bcc9e..5829087f97 100644 --- a/scripts/release/verify.ts +++ b/scripts/release/verify.ts @@ -9,7 +9,33 @@ import { parseArgs } from 'node:util' import { isEntry } from './process.ts' -import { releaseFamily, type ReleaseFamily, type ReleaseMember } from './families.ts' +import { releaseFamily, type PublishPlan, type ReleaseFamily, type ReleaseMember } from './families.ts' + +/** + * Print the publish order the release will follow, and the peer declarations it + * leaves unordered. + * + * The order is the release's own plan: an interrupted publication leaves exactly + * a prefix of it, so reading it is how anyone judges what a partial run left on + * the registry, and printing it on every pull request is what makes a change to + * the order reviewable rather than only observable during a publication. + * @param family - the release family. + * @param plan - the resolved order and its dropped edges. + */ +function reportPublishOrder(family: ReleaseFamily, plan: PublishPlan): void { + console.log(`release verify: publish order for family ${family.id}, ${String(plan.order.length)} member(s):`) + const width = String(plan.order.length).length + for (const [index, member] of plan.order.entries()) { + console.log(` ${String(index + 1).padStart(width, ' ')} ${member.name}@${member.version}`) + } + if (plan.droppedPeerEdges.length === 0) return + console.log( + `release verify: ${String(plan.droppedPeerEdges.length)} peer declaration(s) publish unordered,` + + ' because the peer cannot precede the package declaring it without contradicting a dependency edge' + + ' or its own cycle. npm treats an unmet peer as a warning, so this orders nothing and blocks nothing:', + ) + for (const edge of plan.droppedPeerEdges) console.log(` ${edge.consumer} -> ${edge.peer}`) +} /** * Assert every member may be published: npm refuses a `private` package. @@ -58,12 +84,13 @@ function main(): void { // Resolve the publish order here, before the build: an install-edge cycle // makes the order unrepresentable, and that has to surface at the first gate // rather than when pack is already writing tarballs. - const ordered = family.publishOrder(members) - if (ordered.length !== members.length) { + const plan = family.publishOrder(members) + if (plan.order.length !== members.length) { throw new Error( - `release family ${family.id}: publish order covers ${String(ordered.length)} of ${String(members.length)} members`, + `release family ${family.id}: publish order covers ${String(plan.order.length)} of ${String(members.length)} members`, ) } + reportPublishOrder(family, plan) const publishing = process.env.RELEASE_PUBLISH === 'true' if (publishing) { @@ -73,7 +100,11 @@ function main(): void { const versions = [...new Set(members.map(member => member.version))] const summary = versions.length === 1 ? versions[0] : `${String(versions.length)} versions` - console.log(`release verify: family ${family.id}, ${String(members.length)} member(s), ${summary}, publish order resolved${publishing ? ', publish gates passed' : ''}`) + console.log( + `release verify: family ${family.id}, ${String(members.length)} member(s), ${summary},` + + ` publish order resolved, ${String(plan.droppedPeerEdges.length)} peer declaration(s) unordered` + + (publishing ? ', publish gates passed' : ''), + ) } if (isEntry(import.meta.url)) main() From 7b973e27c807b4e4ece13329e74a5390d091d45e Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:08:01 +0800 Subject: [PATCH 4/6] feat(release): reject a module-scope load of an optional dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dependency in optionalDependencies, or a peer carrying peerDependenciesMeta..optional, may be absent from an installed tree — that absence is the whole promise of "optional". A static import is evaluated when the importing module loads, so one absent package stops being "this capability is unavailable" and becomes a load failure for everything that reaches the importing module. Nothing checked it, and nothing here could: the failure needs an installed tree missing that package, and a workspace install always has every package, so the unit tests, the snapshots, and the packed-install probe all pass while the published package is broken for the consumer who declined the optional peer. verify-optional-dependency-imports reads each package's own manifest for what it allows to be absent, then scans the files that ship across both compiler faces. Value-versus-type is decided against a bound Program rather than the import syntax, because verbatimModuleSyntax is off: the compiler already erases an import whose bindings resolve to types, so a syntactic rule would report four forms that emit nothing. Only the type phase erases an import — `import defer` still resolves and links its module, deferring evaluation alone — which is what phaseModifier expresses and the deprecated isTypeOnly cannot. A violation names the package, the declaration that made it optional, and the way out in order: import it as a type, or restructure so module scope does not need it. A dynamic import() only moves the failure to first use, so the gate does not offer it as the remedy. The gate runs in ci-static and ci-primary through ciSharedStaticGates and locally in hygiene; it needs no build. TypeScriptProject gained a face parameter so a repository-wide gate can seed the client aggregate, which was previously unreachable; the constraint it was built with is unchanged, a face config and never the root solution. The tree has no violation today, so this guards the rule rather than fixing a defect. The spec pins all seven import forms against what tsc emits, including the four a syntactic rule would misreport. --- ...2026-08-10-npm-release-sequences.i18n.yaml | 4 +- .../2026-08-10-npm-release-sequences.md | 8 + .../2026-08-10-npm-release-sequences.zh.md | 8 + package.json | 3 +- scripts/run-gates.ts | 6 + scripts/ts-project.ts | 20 +- ...verify-optional-dependency-imports.spec.ts | 130 +++++++++++ scripts/verify-optional-dependency-imports.ts | 214 ++++++++++++++++++ 8 files changed, 385 insertions(+), 8 deletions(-) create mode 100644 scripts/verify-optional-dependency-imports.spec.ts create mode 100644 scripts/verify-optional-dependency-imports.ts diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml index 4b14d3b8aa..851f0e9d2d 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml @@ -2,5 +2,5 @@ # 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-10-npm-release-sequences.md -2026-08-10-npm-release-sequences.md: 2c46fb9b3e3fb8ddd90131e3c3113166580608d4 -2026-08-10-npm-release-sequences.zh.md: edbb2a8884f658b6c87ceb5762c01551a81a249d +2026-08-10-npm-release-sequences.md: d8495f158482d5d6e06a1752a096d1e9200b6070 +2026-08-10-npm-release-sequences.zh.md: 24b466f6b7b10d31ac2e025da6e12ec3c91c7548 diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md index 2c46fb9b3e..d8495f1584 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md @@ -80,6 +80,14 @@ Every reference to a workspace member uses `workspace:^`, so `pnpm pack` substit `scripts/check-workspace-constraints.ts` requires the protocol, so a new package cannot reintroduce a hand-written range; the invariant-companion rule requires `workspace:^` for `@deepseek-ai/dsh-invariants` for the same reason. +### An optional dependency is never loaded at module scope + +A dependency in `optionalDependencies`, or a peer carrying `peerDependenciesMeta..optional`, may be absent from an installed tree — that absence is the whole promise of "optional". A static import is evaluated when the importing module loads, so one absent package stops being "this capability is unavailable" and becomes a load failure for everything that reaches the importing module. The failure appears only in an installed tree missing that package, and no test here constructs one: a workspace install always has every package, so the unit tests, the snapshots, and the packed-install probe all pass while the published package is broken for the consumer who declined the optional peer. + +[`verify-optional-dependency-imports`](../../../../scripts/verify-optional-dependency-imports.ts) closes that hole. It reads each package's own manifest for what that package allows to be absent, then scans the files that ship — `packages/*/*/src/` and `apps/*/src/` — across both compiler faces. `vendor/` is out of scope, as pinned upstream source under the [vendoring policy](../../../../vendor/README.md). Value-versus-type is decided against a bound Program rather than the import syntax, because `verbatimModuleSyntax` is off: the compiler already erases an import whose bindings resolve to types, so `import type {}`, `import {}`, an inline `type` specifier, and a named binding that resolves to a type all emit nothing and are allowed, while a bare import, a value binding, and a star re-export are kept and rejected. Only the type phase erases an import: `import defer` still resolves and links its module, deferring evaluation alone, so the gate counts it as a load. + +A violation names the package, the declaration that made it optional, and the way out in order — import it as a type, which is all that declaration merging needs, or restructure so module scope does not need the package. A dynamic `import()` only moves the failure to first use, so it belongs to a caller that genuinely requires the package and handles its absence; reaching for it is a sign the dependency is not optional, and the gate does not offer it as the remedy. + ### Release family objects The entity in this domain is a **release family**: a set of packages sharing one version baseline and tag naming that publishes as a unit. Adding a family means adding a subclass and a workflow lane, not changing the core. diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md index edbb2a8884..24b466f6b7 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md @@ -80,6 +80,14 @@ registry 的两个行为决定了「怎么尝试一次发布」。写入之间 `scripts/check-workspace-constraints.ts` 要求这个协议,所以新包无法再引入硬写的范围;同理,invariant companion 规则要求 `@deepseek-ai/dsh-invariants` 用 `workspace:^`。 +### optional 依赖绝不在模块作用域被加载 + +`optionalDependencies` 里的依赖,或带 `peerDependenciesMeta..optional` 的 peer,在安装出来的树里可以不存在——这份「可以不存在」正是 optional 的全部承诺。而静态 import 在引入方模块加载时就求值,于是一个缺失的包不再表现为「这个能力不可用」,而是变成所有能走到该模块的代码的加载失败。这种失败只在「缺了该包的安装树」里出现,而本仓没有任何测试构造这种树:workspace 安装总是把每个包都装上,所以单测、快照、打包安装探针全都会过,而那个拒绝了这个 optional peer 的消费者拿到的却是坏的包。 + +[`verify-optional-dependency-imports`](../../../../scripts/verify-optional-dependency-imports.ts) 堵掉这个洞。它从每个包自己的 manifest 读取「这个包允许谁缺失」,再扫描会发布出去的文件——`packages/*/*/src/` 与 `apps/*/src/`——且两个编译门面各扫一遍。`vendor/` 不在范围内,那是[受 vendoring 政策管辖](../../../../vendor/README.md)的固定上游源码。值与类型的判定对着绑定好的 Program 做,而不是看 import 写法,因为 `verbatimModuleSyntax` 是关的:编译器本来就会消除绑定解析为类型的 import,所以 `import type {}`、`import {}`、内联 `type` 说明符、以及解析为类型的具名绑定都不产生产物、一律放行,而裸 import、值绑定、星号 re-export 会被保留、一律报错。只有 type 相位会消除 import:`import defer` 仍然解析并链接它的模块,只推迟求值,所以门禁把它算作一次加载。 + +报错会点名这个包、点名是哪条声明把它标成 optional 的,并按顺序给出出路——把它作为类型引入(声明合并需要的仅此而已),或者调整写法让模块作用域不再需要这个包。动态 `import()` 只是把失败推迟到首次使用,它属于那种确实需要这个包、并且自己处理缺失的调用方;会想到它,往往说明这个依赖并不 optional,所以门禁不把它作为解法给出。 + ### 发布族对象 这个领域里的实体是**发布族**:一组共享版本基线与 tag 命名、可整体发布的包。新增一族等于加一个子类和一条 workflow lane,不改核心。 diff --git a/package.json b/package.json index 1fd63bae9a..517d0c56d1 100644 --- a/package.json +++ b/package.json @@ -95,6 +95,7 @@ "website:build": "pnpm run docs:build", "verify-package-readme-limitations": "tsx scripts/verify-package-readme-limitations.ts", "verify-node-next-types": "tsx scripts/verify-node-next-types.ts", + "verify-optional-dependency-imports": "tsx scripts/verify-optional-dependency-imports.ts", "verify-runtime-closure": "tsx scripts/verify-runtime-closure.ts", "verify-vendored-links": "tsx scripts/verify-vendored-links.ts", "verify-cordis-config": "tsx scripts/verify-cordis-config.ts", @@ -125,7 +126,7 @@ "verify-module-graph": "tsx scripts/gen-module-graph.ts --check", "constraints": "tsx scripts/check-workspace-constraints.ts", "doc-sync": "tsx scripts/run-gates.ts doc-sync", - "hygiene": "pnpm run rescope-vendor:check && pnpm run knip && pnpm run publint && pnpm run constraints && pnpm run verify-dsh-package-licenses && pnpm run verify-package-invariants && pnpm run verify-built-package-invariants && pnpm run verify-cordis-config && pnpm run verify-node-next-types && pnpm run verify-runtime-closure && pnpm run verify-vendored-links", + "hygiene": "pnpm run rescope-vendor:check && pnpm run knip && pnpm run publint && pnpm run constraints && pnpm run verify-dsh-package-licenses && pnpm run verify-package-invariants && pnpm run verify-built-package-invariants && pnpm run verify-cordis-config && pnpm run verify-node-next-types && pnpm run verify-optional-dependency-imports && pnpm run verify-runtime-closure && pnpm run verify-vendored-links", "publish:npm-baseline": "tsx scripts/publish-npm-baseline.ts", "release:dsh": "tsx scripts/release/bump.ts --family dsh", "release:vendor": "tsx scripts/release/bump.ts --family vendor", diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index 27664fab5e..6b775c9bf1 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -249,6 +249,9 @@ function ciSharedStaticGates(): Gate[] { pnpmScript('dsh-package-licenses', 'verify-dsh-package-licenses', { label: 'DSH package licenses' }), pnpmScript('package-invariants', 'verify-package-invariants', { label: 'package invariants' }), pnpmScript('cordis-config', 'verify-cordis-config', { label: 'Cordis config' }), + pnpmScript('optional-dependency-imports', 'verify-optional-dependency-imports', { + label: 'optional dependency imports', + }), pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), ] } @@ -565,6 +568,9 @@ function hygieneLeafGates(options: { artifactNeeds?: string[] } = {}): Gate[] { label: 'node-next types', ...artifactOptions, }), + pnpmScript('optional-dependency-imports', 'verify-optional-dependency-imports', { + label: 'optional dependency imports', + }), ] } diff --git a/scripts/ts-project.ts b/scripts/ts-project.ts index 9a0400a39d..53b100ceb0 100644 --- a/scripts/ts-project.ts +++ b/scripts/ts-project.ts @@ -11,6 +11,12 @@ interface ProjectGraph { options: ts.CompilerOptions } +/** + * A compiler face: the two aggregates a repository-wide program may seed from. + * The root solution is never one of them. + */ +export type CompilerFace = 'host' | 'client' + /** TypeScript config host shared by repository scripts. */ export const repositoryConfigHost: ts.ParseConfigFileHost = { useCaseSensitiveFileNames: ts.sys.useCaseSensitiveFileNames, @@ -24,12 +30,12 @@ export const repositoryConfigHost: ts.ParseConfigFileHost = { } /** - * Parse the host aggregate tsconfig and flatten all referenced projects into one + * Parse one face aggregate tsconfig and flatten all referenced projects into one * semantic graph. Never seed the root solution: flattening host+client into one * program collides the cordis Context merges. */ -function loadProjectGraph(projectRoot: string): ProjectGraph { - const rootConfigPath = resolve(projectRoot, 'tsconfig.host.json') +function loadProjectGraph(projectRoot: string, face: CompilerFace): ProjectGraph { + const rootConfigPath = resolve(projectRoot, `tsconfig.${face}.json`) const rootConfig = parseConfig(rootConfigPath) const rootNames = new Set() const visited = new Set() @@ -81,8 +87,12 @@ export class TypeScriptProject { /** The checker shared by every semantic query in this project. */ readonly checker: ts.TypeChecker - constructor(private readonly projectRoot: string) { - const graph = loadProjectGraph(projectRoot) + /** + * @param projectRoot - repository root the program is seeded and reported from. + * @param face - which compiler face aggregate to flatten. + */ + constructor(readonly projectRoot: string, face: CompilerFace = 'host') { + const graph = loadProjectGraph(projectRoot, face) this.program = ts.createProgram(graph.rootNames, semanticCompilerOptions(graph.options)) this.checker = this.program.getTypeChecker() } diff --git a/scripts/verify-optional-dependency-imports.spec.ts b/scripts/verify-optional-dependency-imports.spec.ts new file mode 100644 index 0000000000..3bb857050f --- /dev/null +++ b/scripts/verify-optional-dependency-imports.spec.ts @@ -0,0 +1,130 @@ +/** + * Tests for the optional-dependency load gate: which import and re-export forms + * survive emit, and therefore load a package the installed tree may not carry. + * + * The expectations here match what `tsc` emits with `verbatimModuleSyntax` off: + * `import type`, `import {}`, an inline `type` specifier, and a named binding + * that resolves to a type all disappear; a bare import, a value binding, and a + * star re-export remain. + */ + +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import { TypeScriptProject } from './ts-project.ts' +import { collectOptionalImportViolations } from './verify-optional-dependency-imports.ts' + +const FIXTURE: Record = { + 'tsconfig.host.json': JSON.stringify({ + compilerOptions: { + target: 'es2022', + module: 'esnext', + moduleResolution: 'bundler', + noEmit: true, + skipLibCheck: true, + types: [], + paths: { + '@f/opt': ['./packages/f/opt/src/index.ts'], + '@f/hard': ['./packages/f/hard/src/index.ts'], + }, + }, + include: ['packages/**/*.ts'], + }), + + 'packages/f/opt/package.json': JSON.stringify({ name: '@f/opt', version: '0.0.1' }), + 'packages/f/opt/src/index.ts': [ + 'export interface Shape { a: number }', + 'export const runtimeValue = 1', + '', + ].join('\n'), + + 'packages/f/hard/package.json': JSON.stringify({ name: '@f/hard', version: '0.0.1' }), + 'packages/f/hard/src/index.ts': 'export const hardValue = 2\n', + + // The consumer allows @f/opt to be absent and requires @f/hard. + 'packages/f/consumer/package.json': JSON.stringify({ + name: '@f/consumer', + version: '0.0.1', + dependencies: { '@f/hard': '*' }, + peerDependencies: { '@f/opt': '*' }, + peerDependenciesMeta: { '@f/opt': { optional: true } }, + }), + + // Elided by the compiler, so each of these is allowed. + 'packages/f/consumer/src/allowed-type-only.ts': [ + "import type {} from '@f/opt'", + 'export const a = 1', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-empty.ts': [ + "import {} from '@f/opt'", + 'export const b = 1', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-inline-type.ts': [ + "import { type Shape } from '@f/opt'", + 'export const c: Shape = { a: 1 }', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-type-binding.ts': [ + "import { Shape } from '@f/opt'", + 'export const d: Shape = { a: 1 }', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-type-reexport.ts': [ + "export type { Shape } from '@f/opt'", + '', + ].join('\n'), + // A hard dependency may be loaded at module scope; only optional ones may not. + 'packages/f/consumer/src/allowed-hard-dependency.ts': [ + "import { hardValue } from '@f/hard'", + 'export const e = hardValue', + '', + ].join('\n'), + + // Kept by the compiler, so each of these loads a package that may be absent. + 'packages/f/consumer/src/rejected-bare.ts': [ + "import '@f/opt'", + 'export const f = 1', + '', + ].join('\n'), + 'packages/f/consumer/src/rejected-value.ts': [ + "import { runtimeValue } from '@f/opt'", + 'export const g = runtimeValue', + '', + ].join('\n'), + 'packages/f/consumer/src/rejected-star-reexport.ts': [ + "export * from '@f/opt'", + '', + ].join('\n'), +} + +const root = mkdtempSync(join(tmpdir(), 'optional-imports-')) +for (const [rel, content] of Object.entries(FIXTURE)) { + mkdirSync(dirname(join(root, rel)), { recursive: true }) + writeFileSync(join(root, rel), content) +} +const violations = collectOptionalImportViolations(new TypeScriptProject(root)) + +afterAll(() => { + rmSync(root, { recursive: true, force: true }) +}) + +describe('optional dependency loads', () => { + it('reports every form the compiler keeps, and nothing else', () => { + expect(violations.map(violation => violation.split(' loads ')[0])).toEqual([ + 'packages/f/consumer/src/rejected-bare.ts:1', + 'packages/f/consumer/src/rejected-star-reexport.ts:1', + 'packages/f/consumer/src/rejected-value.ts:1', + ]) + }) + + it('names the package, the declaration that made it optional, and the way out', () => { + expect(violations[0]).toBe( + 'packages/f/consumer/src/rejected-bare.ts:1 loads @f/opt at module scope,' + + ' declared optional in peerDependenciesMeta; import it as a type,' + + ' or restructure so module scope does not need it', + ) + }) +}) diff --git a/scripts/verify-optional-dependency-imports.ts b/scripts/verify-optional-dependency-imports.ts new file mode 100644 index 0000000000..e7e16d7aab --- /dev/null +++ b/scripts/verify-optional-dependency-imports.ts @@ -0,0 +1,214 @@ +/** + * Reject a static value import of an optional dependency. + * + * A dependency declared in `optionalDependencies`, or as a peer carrying + * `peerDependenciesMeta..optional`, may be absent from an installed tree — + * that absence is what "optional" promises a consumer. A static import is + * evaluated when the importing module loads, so one absent package turns + * "this capability is unavailable" into a load failure for everything that + * reaches the importing module. + * + * The way out, in order: import it as a type, which emits nothing and is all + * that declaration merging needs; or restructure so nothing at module scope + * needs the package. A dynamic `import()` only moves the failure to first use, + * so it belongs to a caller that genuinely requires the package and handles its + * absence — it is a last resort, not the default answer, and reaching for it is + * a sign the dependency is not optional. + * + * Value-vs-type is decided against a bound Program rather than the import + * syntax, because `verbatimModuleSyntax` is off: a named import used only in + * type positions is elided and does not load anything. The decision is + * deliberately conservative in one direction — a value binding the compiler + * would elide because nothing references it in a value position is still + * reported, and the fix it asks for (`import type`, or dropping the binding) is + * what the published package wants regardless. Both compiler faces are scanned, + * and only files that ship — a published package's `src` — are subject. + */ + +import { existsSync, readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import ts from 'typescript' +import { TypeScriptProject, type CompilerFace } from './ts-project.ts' + +const root = resolve(import.meta.dirname, '..') + +/** Directories whose `src` ships as a published package. */ +const PUBLISHED_SOURCE = /^(?:packages\/[^/]+\/[^/]+|apps\/[^/]+)\/src\// + +/** How a manifest marked a dependency optional, for the violation message. */ +type OptionalKind = 'optionalDependencies' | 'peerDependenciesMeta' + +/** + * The package name a module specifier resolves to. + * @param specifier - an import specifier, possibly a subpath. + * @returns The bare package name, keeping a leading scope. + */ +function packageOf(specifier: string): string { + const parts = specifier.split('/') + return specifier.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0] ?? specifier +} + +/** + * Read a manifest field as a record. + * @param manifest - parsed manifest. + * @param field - field name. + * @returns The field value, or an empty record. + */ +function record(manifest: Record, field: string): Record { + const value = manifest[field] + if (value === null || typeof value !== 'object' || Array.isArray(value)) return {} + return value as Record +} + +/** + * The dependencies one manifest allows to be absent. + * @param manifest - parsed manifest. + * @returns Each optional package name and how it was marked. + */ +function optionalDependencies(manifest: Record): Map { + const optional = new Map() + for (const name of Object.keys(record(manifest, 'optionalDependencies'))) { + optional.set(name, 'optionalDependencies') + } + const peers = record(manifest, 'peerDependencies') + for (const [name, meta] of Object.entries(record(manifest, 'peerDependenciesMeta'))) { + if (meta === null || typeof meta !== 'object') continue + if ((meta as Record).optional !== true) continue + // A meta entry for an undeclared peer is check-workspace-constraints' business. + if (!(name in peers)) continue + optional.set(name, 'peerDependenciesMeta') + } + return optional +} + +/** One package directory's optional dependencies, resolved once per directory. */ +const optionalByDirectory = new Map>() + +/** + * The optional dependencies of the package owning a source file. + * @param projectRoot - root the relative path is resolved against. + * @param relativePath - repository-relative path of a source file. + * @returns That package's optional dependencies, empty when it declares none. + */ +function optionalFor(projectRoot: string, relativePath: string): Map { + const directory = resolve(projectRoot, relativePath.slice(0, relativePath.indexOf('/src/'))) + const cached = optionalByDirectory.get(directory) + if (cached !== undefined) return cached + const manifestPath = resolve(directory, 'package.json') + const parsed: unknown = existsSync(manifestPath) ? JSON.parse(readFileSync(manifestPath, 'utf8')) : {} + const manifest = parsed !== null && typeof parsed === 'object' && !Array.isArray(parsed) + ? parsed as Record + : {} + const optional = optionalDependencies(manifest) + optionalByDirectory.set(directory, optional) + return optional +} + +/** + * Whether one binding of an import or re-export names a value. + * @param name - the local binding name node. + * @param checker - the program's checker. + * @returns True when the binding carries value meaning, and on an unresolved + * symbol, so an unresolvable binding fails closed. + */ +function bindsValue(name: ts.Identifier | ts.StringLiteral, checker: ts.TypeChecker): boolean { + const symbol = checker.getSymbolAtLocation(name) + if (symbol === undefined) return true + const target = (symbol.flags & ts.SymbolFlags.Alias) === 0 ? symbol : checker.getAliasedSymbol(symbol) + return (target.flags & ts.SymbolFlags.Value) !== 0 +} + +/** + * Whether an import declaration loads its module at run time. + * @param declaration - the import declaration. + * @param checker - the program's checker. + * @returns True when the emitted module keeps the import. + */ +function importLoadsModule(declaration: ts.ImportDeclaration, checker: ts.TypeChecker): boolean { + const clause = declaration.importClause + // A bare `import 'x'` is kept for its side effects. + if (clause === undefined) return true + // Only the type phase erases the import. `import defer` still resolves and + // links the module, deferring evaluation alone, so an absent package fails + // exactly as it would without the modifier. + if (clause.phaseModifier === ts.SyntaxKind.TypeKeyword) return false + if (clause.name !== undefined) return true + const bindings = clause.namedBindings + if (bindings === undefined || ts.isNamespaceImport(bindings)) return true + return bindings.elements.some(element => !element.isTypeOnly && bindsValue(element.name, checker)) +} + +/** + * Whether a re-export loads its module at run time. + * @param declaration - the export declaration, which carries a module specifier. + * @param checker - the program's checker. + * @returns True when the emitted module keeps the re-export. + */ +function exportLoadsModule(declaration: ts.ExportDeclaration, checker: ts.TypeChecker): boolean { + if (declaration.isTypeOnly) return false + const clause = declaration.exportClause + // `export * from 'x'` re-exports whatever values the module has. + if (clause === undefined || ts.isNamespaceExport(clause)) return true + return clause.elements.some(element => !element.isTypeOnly && bindsValue(element.name, checker)) +} + +/** + * Collect every static value import of an optional dependency in one face. + * @param project - a bound repository project. + * @returns One message per violation, sorted by location. + */ +export function collectOptionalImportViolations(project: TypeScriptProject): string[] { + const checker = project.checker + const violations: string[] = [] + for (const sourceFile of project.sourceFiles()) { + if (sourceFile.isDeclarationFile) continue + const relativePath = project.relativePath(sourceFile) + if (!PUBLISHED_SOURCE.test(relativePath)) continue + const optional = optionalFor(project.projectRoot, relativePath) + if (optional.size === 0) continue + + for (const statement of sourceFile.statements) { + const isImport = ts.isImportDeclaration(statement) + if (!isImport && !ts.isExportDeclaration(statement)) continue + const specifierNode = statement.moduleSpecifier + if (specifierNode === undefined || !ts.isStringLiteral(specifierNode)) continue + const kind = optional.get(packageOf(specifierNode.text)) + if (kind === undefined) continue + const loads = isImport + ? importLoadsModule(statement, checker) + : exportLoadsModule(statement, checker) + if (!loads) continue + const { line } = sourceFile.getLineAndCharacterOfPosition(statement.getStart(sourceFile)) + violations.push( + `${relativePath}:${String(line + 1)} loads ${specifierNode.text} at module scope,` + + ` declared optional in ${kind}; import it as a type, or restructure so module scope does not need it`, + ) + } + } + return violations.sort((left, right) => left.localeCompare(right)) +} + +/** CLI entry: list every violation and exit 1, or confirm the invariant holds. */ +function main(): void { + const faces: readonly CompilerFace[] = ['host', 'client'] + const violations = new Set() + for (const face of faces) { + for (const violation of collectOptionalImportViolations(new TypeScriptProject(root, face))) { + violations.add(violation) + } + } + if (violations.size === 0) { + console.log('verify-optional-dependency-imports: no optional dependency is loaded at module scope.') + return + } + console.error(`verify-optional-dependency-imports: ${String(violations.size)} optional dependency load(s) at module scope:`) + for (const violation of [...violations].sort((left, right) => left.localeCompare(right))) { + console.error(` ${violation}`) + } + process.exit(1) +} + +// Run only when invoked as a script, not when imported by a test. +if (process.argv[1] && import.meta.filename === resolve(process.argv[1])) { + main() +} From 0e50fa290c6a21fb0e4f3b5fd3e4a7807ffcaae4 Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:14:16 +0800 Subject: [PATCH 5/6] fix(release): state what the echo helper does, and drop two dead claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three corrections from review, none of which change behaviour. attemptStreaming promised output "as the command produces it", which spawnSync cannot do: it returns only after the child exits, and the two streams are echoed one after the other, so their interleaving is lost. For an npm publish that is visible — notices go to stderr while the `+ name@version` confirmation goes to stdout, so the confirmation prints first. The helper is now attemptEchoed and its contract says buffered, echoed after exit, stdout before stderr; live progress would need an asynchronous spawn with data listeners. The traversal comment claimed a node on the stack is a cycle only peer edges can form, and that skipping it drops just that edge. The cycle does carry a peer edge, because the install edges were proved acyclic a moment earlier, but the back edge that reaches the stacked node need not be the peer one — which is what the post-condition exists to catch, so the comment now points at it instead of asserting an invariant the traversal does not have. The `if (placed.has(member.name)) return` after leaving the stack was unreachable: a re-entrant visit returns at the top guard while the member is on the stack, so it can never be placed by the time the recursion unwinds. --- scripts/release/families.ts | 8 +++++--- scripts/release/process.ts | 18 +++++++++++++----- scripts/release/publish.ts | 4 ++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/scripts/release/families.ts b/scripts/release/families.ts index 09acbe2da9..f43939f17d 100644 --- a/scripts/release/families.ts +++ b/scripts/release/families.ts @@ -180,8 +180,11 @@ export abstract class ReleaseFamily { } for (const member of byNameSorted) checkInstall(member, []) - // Emit the order over both kinds of edge. A node already on the stack is a - // cycle only peer edges can form, and skipping it drops just that edge. + // Emit the order over both kinds of edge. A node already on the stack closes + // a cycle, and that cycle carries at least one peer edge because the install + // edges were just proved acyclic — but the back edge that reaches the stacked + // node is not necessarily the peer one, so the post-condition below decides + // whether the emitted order survived. const ordered: ReleaseMember[] = [] const droppedPeerEdges: DroppedPeerEdge[] = [] const placed = new Set() @@ -216,7 +219,6 @@ export abstract class ReleaseFamily { visit(peer) } onStack.delete(member.name) - if (placed.has(member.name)) return placed.add(member.name) ordered.push(member) } diff --git a/scripts/release/process.ts b/scripts/release/process.ts index acec98feae..392a6cab66 100644 --- a/scripts/release/process.ts +++ b/scripts/release/process.ts @@ -39,17 +39,25 @@ export function attempt(command: string, args: readonly string[], options: RunOp } /** - * Run a command, letting its output reach the log while also returning it. + * Run a command, capture its output, and echo it once the command exits. * - * A step that both shows progress and classifies its own failure needs both: the - * output has to appear in the workflow log as the command produces it, and the - * caller has to read it to decide whether a failure is worth retrying. + * A step that both shows what a command said and classifies its own failure + * needs both halves: the output has to reach the workflow log, and the caller has + * to read it to decide whether a failure is worth retrying. + * + * This is not live progress. `spawnSync` returns only after the child exits, so + * nothing appears while the command runs, and the two streams are echoed one + * after the other — all of stdout, then all of stderr — which loses their + * interleaving. For an npm publish that matters in one visible way: `npm notice` + * lines go to stderr while the `+ name@version` confirmation goes to stdout, so + * the log shows the confirmation first. Live progress would need an + * asynchronous spawn with data listeners. * @param command - executable name. * @param args - command arguments. * @param options - working directory and environment. * @returns The exit status and captured streams. */ -export function attemptStreaming(command: string, args: readonly string[], options: RunOptions = {}): CommandResult { +export function attemptEchoed(command: string, args: readonly string[], options: RunOptions = {}): CommandResult { const result = spawnSync(command, [...args], { cwd: options.cwd, env: options.env, diff --git a/scripts/release/publish.ts b/scripts/release/publish.ts index f861da18c2..6301241ed7 100644 --- a/scripts/release/publish.ts +++ b/scripts/release/publish.ts @@ -18,7 +18,7 @@ import { join, resolve } from 'node:path' import { setTimeout as sleep } from 'node:timers/promises' import { parseArgs } from 'node:util' import { releaseFamily } from './families.ts' -import { attempt, attemptStreaming, isEntry } from './process.ts' +import { attempt, attemptEchoed, isEntry } from './process.ts' import { packedIdentity, readPublishOrder } from './tarball.ts' /** @@ -102,7 +102,7 @@ async function publishTarball(tarball: string, name: string, version: string): P // command-line flag could not serve both and would override the manifest // that does. Each packed manifest decides, and // check-workspace-constraints holds every manifest to its sequence's level. - const result = attemptStreaming('npm', ['publish', tarball, ...tagArgs]) + const result = attemptEchoed('npm', ['publish', tarball, ...tagArgs]) const output = `${result.stdout}${result.stderr}` if (result.status === 0) return From d5be1d62c9e565399b44e07ac0caf4500a817439 Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:19:02 +0800 Subject: [PATCH 6/6] feat(release): count publish progress against the whole release set A dsh publication uploads 221 packages one at a time, spaced apart, and the log gave no way to tell how far along a run was: every line named a package, none said where that package sat in the set. Each per-package line now carries [n/total]. Every entry in the order settles as either published or already present, so the counter is both "packages settled" and "position in the publish order", and the closing summary names the member count alongside the published and skipped totals. --- scripts/release/publish.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/scripts/release/publish.ts b/scripts/release/publish.ts index 6301241ed7..2590e96e52 100644 --- a/scripts/release/publish.ts +++ b/scripts/release/publish.ts @@ -136,9 +136,15 @@ async function main(): Promise { const family = releaseFamily(values.family) const directory = resolve(process.cwd(), values.from) + // Every entry in the order settles as either published or already present, so + // one counter answers "how far along is this run" for whoever is watching a + // release that takes minutes per family. + const order = readPublishOrder(directory) + const total = String(order.length) let published = 0 let skipped = 0 - for (const filename of readPublishOrder(directory)) { + for (const [index, filename] of order.entries()) { + const progress = `[${String(index + 1)}/${total}]` const tarball = join(directory, filename) const { name, version } = packedIdentity(tarball) const state = registryState(name, version) @@ -151,7 +157,7 @@ async function main(): Promise { + '\nBump the version, or investigate why the build is not reproducible.', ) } - console.log(`release publish: ${name}@${version} already published, skipping`) + console.log(`release publish: ${progress} ${name}@${version} already published, skipping`) skipped += 1 continue } @@ -159,11 +165,14 @@ async function main(): Promise { // only skips does not wait at all. if (published > 0) await sleep(PUBLISH_SPACING_MS) await publishTarball(tarball, name, version) - console.log(`release publish: ${name}@${version} published`) + console.log(`release publish: ${progress} ${name}@${version} published`) published += 1 } - console.log(`release publish: family ${family.id}, ${String(published)} published, ${String(skipped)} already present`) + console.log( + `release publish: family ${family.id}, ${total} member(s),` + + ` ${String(published)} published, ${String(skipped)} already present`, + ) } if (isEntry(import.meta.url)) await main()