mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(review): label host-preparation failures and gate vendored lockfile links
ds-review-bot round 1: boot() now throws `host preparation failed` when prepare() rejects before any config-tree entry mounts (the plugin-tree label overstated), and the new hygiene gate verify-vendored-links pins the linkWorkspacePackages fix — every vendored package name in pnpm-lock.yaml must resolve to a workspace link with no registry copy alongside.
This commit is contained in:
@@ -77,6 +77,7 @@
|
||||
"verify-package-readme-limitations": "tsx scripts/verify-package-readme-limitations.ts",
|
||||
"verify-node-next-types": "tsx scripts/verify-node-next-types.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",
|
||||
"verify-client-domain-graph": "tsx scripts/verify-client-domain-graph.ts",
|
||||
"gen-cordis-catalog": "tsx scripts/gen-cordis-catalog.ts",
|
||||
@@ -100,7 +101,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 knip && pnpm run publint && pnpm run constraints && 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",
|
||||
"hygiene": "pnpm run knip && pnpm run publint && pnpm run constraints && 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",
|
||||
"dsh": "node --import tsx/esm apps/cli/src/bin.ts",
|
||||
"demo:headless": "node --import tsx packages/examples/cli-demo/src/bin.ts --config examples/headless-agent/cordis.yml",
|
||||
"demo:tui": "node --import tsx/esm apps/cli/src/bin.ts",
|
||||
|
||||
@@ -445,7 +445,9 @@ export async function assertEntriesActivated(ctx: Context, binName: string): Pro
|
||||
* @param prepare - optional host setup run after Loader installation and before any config-tree entry mounts.
|
||||
* @returns the root context once every entry has started, or as soon as a
|
||||
* surface disposed the tree while startup was still in flight.
|
||||
* @throws a labelled load error after disposing the partial context.
|
||||
* @throws a labelled error after disposing the partial context — `host
|
||||
* preparation failed` when `prepare` threw before any config-tree entry
|
||||
* mounted, `plugin tree failed to load` afterwards.
|
||||
*/
|
||||
export async function boot(
|
||||
binName: string,
|
||||
@@ -454,12 +456,16 @@ export async function boot(
|
||||
prepare?: (ctx: Context) => Promise<void> | void,
|
||||
): Promise<Context> {
|
||||
const ctx = new Context()
|
||||
// Two failure labels: `prepare` runs before any config-tree entry mounts,
|
||||
// so its failure is host setup, not the plugin tree.
|
||||
let stage = 'host preparation failed'
|
||||
try {
|
||||
ctx.baseUrl = pathToFileURL(dirname(absoluteConfigPath)).href + '/'
|
||||
ctx.provide('dshHomePath', dshHomePath)
|
||||
await ctx.plugin(Loader)
|
||||
ctx.loader.builtins.include = Include
|
||||
await prepare?.(ctx)
|
||||
stage = 'plugin tree failed to load'
|
||||
// Pinned id: the bootstrap include is app glue, not a config row, and its
|
||||
// id appears in Loader failure chains — a random id would make startup
|
||||
// diagnostics unstable across runs (and snapshot fixtures).
|
||||
@@ -485,6 +491,9 @@ export async function boot(
|
||||
await assertEntriesActivated(ctx, binName)
|
||||
return ctx
|
||||
} catch (cause) {
|
||||
// Root-fiber disposal contains cleanup failures per observer (Cordis
|
||||
// fiber.ts hardening) and a repeated call returns the settled single-shot
|
||||
// result, so this await cannot reject and replace `cause`.
|
||||
await ctx.fiber.dispose()
|
||||
const detail = cause instanceof Error ? cause.message : String(cause)
|
||||
// The transactional Loader wraps a failing entry apply in one message per
|
||||
@@ -495,7 +504,7 @@ export async function boot(
|
||||
let deepest: unknown = cause
|
||||
while (deepest instanceof Error && deepest.cause !== undefined) deepest = deepest.cause
|
||||
const stack = deepest instanceof Error && deepest !== cause ? `\n${deepest.stack ?? deepest.message}` : ''
|
||||
throw new Error(`${binName}: plugin tree failed to load: ${detail}${stack}`, { cause })
|
||||
throw new Error(`${binName}: ${stage}: ${detail}${stack}`, { cause })
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -335,7 +335,7 @@ describe('boot', () => {
|
||||
})
|
||||
|
||||
await expect(task).rejects.toMatchObject({
|
||||
message: `${NAME}: plugin tree failed to load: ${failure}`,
|
||||
message: `${NAME}: host preparation failed: ${failure}`,
|
||||
cause: failure,
|
||||
})
|
||||
expect(disposed).toBe(true)
|
||||
@@ -418,9 +418,9 @@ describe('boot', () => {
|
||||
const deepest = new Error('stackless deep failure')
|
||||
delete (deepest as { stack?: string }).stack
|
||||
await expect(boot(NAME, join(dir, 'cordis.yml'), undefined, () => {
|
||||
throw new Error('host preparation failed', { cause: deepest })
|
||||
throw new Error('wrapped setup failure', { cause: deepest })
|
||||
})).rejects.toThrow(
|
||||
`${NAME}: plugin tree failed to load: host preparation failed\nstackless deep failure`,
|
||||
`${NAME}: host preparation failed: wrapped setup failure\nstackless deep failure`,
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
72
scripts/verify-vendored-links.ts
Normal file
72
scripts/verify-vendored-links.ts
Normal file
@@ -0,0 +1,72 @@
|
||||
/**
|
||||
* Verify that pnpm-lock.yaml resolves every vendored package name to its
|
||||
* workspace `link:` — never a registry copy. `linkWorkspacePackages: true`
|
||||
* (pnpm-workspace.yaml) makes matching upstream semver ranges resolve to the
|
||||
* pinned vendored sources; a registry copy of the same name coexisting with
|
||||
* the vendored one silently forks the framework layer (vendor/README.md).
|
||||
*/
|
||||
import { readdir, readFile } from 'node:fs/promises'
|
||||
import { join, resolve } from 'node:path'
|
||||
import * as yaml from 'js-yaml'
|
||||
|
||||
const root = resolve(import.meta.dirname, '..')
|
||||
|
||||
async function vendoredNames(): Promise<Set<string>> {
|
||||
const names = new Set<string>()
|
||||
for (const entry of await readdir(join(root, 'vendor'), { withFileTypes: true })) {
|
||||
if (!entry.isDirectory()) continue
|
||||
let manifest: { name?: string }
|
||||
try {
|
||||
manifest = JSON.parse(await readFile(join(root, 'vendor', entry.name, 'package.json'), 'utf8')) as { name?: string }
|
||||
} catch {
|
||||
continue // not a package directory (e.g. vendor/README.md siblings)
|
||||
}
|
||||
if (manifest.name !== undefined) names.add(manifest.name)
|
||||
}
|
||||
return names
|
||||
}
|
||||
|
||||
interface Lockfile {
|
||||
importers?: Record<string, Record<string, unknown>>
|
||||
packages?: Record<string, unknown>
|
||||
snapshots?: Record<string, unknown>
|
||||
}
|
||||
|
||||
const names = await vendoredNames()
|
||||
if (names.size === 0) throw new Error('verify-vendored-links: no vendored package manifests found under vendor/')
|
||||
const lockfile = yaml.load(await readFile(join(root, 'pnpm-lock.yaml'), 'utf8')) as Lockfile
|
||||
|
||||
const violations: string[] = []
|
||||
|
||||
// Importer resolutions: every dependency entry naming a vendored package must
|
||||
// resolve to a link:, or the build silently uses a registry copy.
|
||||
for (const [importer, sections] of Object.entries(lockfile.importers ?? {})) {
|
||||
for (const [section, dependencies] of Object.entries(sections)) {
|
||||
if (typeof dependencies !== 'object' || dependencies === null) continue
|
||||
for (const [dependency, entry] of Object.entries(dependencies as Record<string, { version?: string }>)) {
|
||||
if (!names.has(dependency)) continue
|
||||
const version = entry.version ?? ''
|
||||
if (!version.startsWith('link:')) {
|
||||
violations.push(`${importer} ${section}.${dependency} resolves to ${JSON.stringify(version)} (expected link:)`)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Package/snapshot keys: a registry copy materializes as a `<name>@<version>`
|
||||
// key; vendored names must never appear there at all.
|
||||
for (const section of ['packages', 'snapshots'] as const) {
|
||||
for (const key of Object.keys(lockfile[section] ?? {})) {
|
||||
const atIndex = key.lastIndexOf('@')
|
||||
if (atIndex <= 0) continue
|
||||
const packageName = key.slice(0, atIndex)
|
||||
if (names.has(packageName)) violations.push(`${section} entry ${key} is a registry copy of a vendored package`)
|
||||
}
|
||||
}
|
||||
|
||||
if (violations.length > 0) {
|
||||
console.error(`verify-vendored-links: ${String(violations.length)} lockfile resolution(s) bypass the vendored workspaces:`)
|
||||
for (const violation of violations) console.error(` - ${violation}`)
|
||||
process.exit(1)
|
||||
}
|
||||
console.log(`verify-vendored-links: all ${String(names.size)} vendored package names resolve to workspace links.`)
|
||||
2
vendor/README.md
vendored
2
vendor/README.md
vendored
@@ -2,7 +2,7 @@
|
||||
|
||||
This directory contains source-vendored copies of the Cordis framework and its foundation libraries. They are copied into this monorepo instead of being depended on via npm, so that the harness fully owns its framework layer (auditable, patchable, pinned).
|
||||
|
||||
All vendored packages keep their **original npm names** and are marked `private: true` — they are never published from this repo. `pnpm-workspace.yaml#linkWorkspacePackages` makes matching upstream semver ranges resolve these pinned workspaces, including imports from built `lib/`; disabling it substitutes npm copies behind the same names. Schemastery's manifest additionally declares a conditional `exports` map (import → `.mjs`, require → `.cjs`): pnpm links the directory itself, so without `exports` Node's ESM resolver would fall back to `main` and load the CJS entry whose lazy `require('cosmokit')` can race ESM loading of the same linked module under module-hook hosts (vitest). Upstream MIT `LICENSE` files are preserved in each package directory.
|
||||
All vendored packages keep their **original npm names** and are marked `private: true` — they are never published from this repo. `pnpm-workspace.yaml#linkWorkspacePackages` makes matching upstream semver ranges resolve these pinned workspaces, including imports from built `lib/`; disabling it substitutes npm copies behind the same names. The `hygiene` gate `verify-vendored-links` asserts every vendored name resolves to a workspace `link:` in `pnpm-lock.yaml` with no registry copy alongside. Schemastery's manifest additionally declares a conditional `exports` map (import → `.mjs`, require → `.cjs`): pnpm links the directory itself, so without `exports` Node's ESM resolver would fall back to `main` and load the CJS entry whose lazy `require('cosmokit')` can race ESM loading of the same linked module under module-hook hosts (vitest). Upstream MIT `LICENSE` files are preserved in each package directory.
|
||||
|
||||
This file covers the manifest, the local-modification log, and the procedure for **updating** an existing vendored package. To **add a new** one, see the cookbook guide: [docs/cookbook/adding-a-vendored-package.md](../docs/cookbook/adding-a-vendored-package.md).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user