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] 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()