فهرست منبع

fix review findings: skip lib/ build-output refs in verify-package-paths; resolve acp built-bin npm deps from the declaring package

verify-package-paths flagged the new built-bin smokes' `lib/bin.js` citations
as stale-source drift, failing CI: doc-sync runs BEFORE build, so the build
output is absent at lint time. The gate targets moved SOURCE paths, so skip any
reference whose target goes through a `lib/` segment — mirroring how the file
scan already excludes `lib/`.

The acp built-bin smoke resolved `zod`/`@agentclientprotocol/sdk` via
`import.meta.resolve` from the test file's own context, but `acp-agent` does not
declare them — `dsh-acp` does. Under pnpm's strict layout they are not exposed
where the test resolves, so the new CI built-bin step failed with
"Cannot find package 'zod'". Resolve each from the `ui/acp` package URL (the one
that declares it) instead.
Tianyi Cui 3 ماه پیش
والد
کامیت
b7d018580e
2فایلهای تغییر یافته به همراه23 افزوده شده و 4 حذف شده
  1. 12 4
      packages/ui/acp-agent/tests/built-bin.e2e.ts
  2. 11 0
      scripts/verify-package-paths.ts

+ 12 - 4
packages/ui/acp-agent/tests/built-bin.e2e.ts

@@ -3,7 +3,7 @@ import { mkdtemp, mkdir, rm, symlink, writeFile, readFile } from 'node:fs/promis
 import { existsSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { dirname, join } from 'node:path'
-import { fileURLToPath } from 'node:url'
+import { fileURLToPath, pathToFileURL } from 'node:url'
 import {
   ClientSideConnection,
   ndJsonStream,
@@ -48,9 +48,14 @@ const vendorPackages = [
   'cordis', 'loader', 'include', 'timer', 'hmr', 'logger-console',
   'schemastery', 'cosmokit',
 ]
-// Third-party deps the ACP bridge needs (resolved from the acp package's own
-// node_modules and linked into the consumer so plain node finds them).
+// Third-party deps the ACP bridge needs at runtime. They are declared by
+// `dsh-acp` (NOT by `acp-agent`), so they live under `packages/ui/acp/node_modules`
+// and are NOT necessarily hoisted where THIS test file can resolve them — pnpm's
+// strict layout only exposes a package's deps under that package. Resolve each
+// from the `ui/acp` package directory (the one that declares it) so the lookup
+// works regardless of hoisting, then symlink it into the consumer for plain node.
 const npmDeps = ['@agentclientprotocol/sdk', 'zod']
+const acpPkgDir = join(repoRoot, 'packages/ui/acp')
 
 async function pkgName(absDir: string): Promise<string> {
   const json = JSON.parse(await readFile(join(absDir, 'package.json'), 'utf8')) as { name: string }
@@ -76,7 +81,10 @@ async function makeConsumer(): Promise<string> {
     await link(abs, await pkgName(abs), nm)
   }
   for (const dep of npmDeps) {
-    const resolved = fileURLToPath(import.meta.resolve(`${dep}/package.json`))
+    // Resolve from `ui/acp`'s package.json URL (the package that declares the
+    // dep), not this test file's location — `acp-agent` does not depend on these.
+    const fromAcp = pathToFileURL(join(acpPkgDir, 'package.json')).href
+    const resolved = fileURLToPath(import.meta.resolve(`${dep}/package.json`, fromAcp))
     await link(dirname(resolved), dep, nm)
   }
   await writeFile(join(dir, 'cordis.yml'), [

+ 11 - 0
scripts/verify-package-paths.ts

@@ -28,6 +28,10 @@
  * Scope mirrors the other doc gates plus repo-authored TypeScript: Markdown
  * across README/docs/packages/AGENTS, and `.ts` under packages/** and
  * examples/** (excluding built `lib/`, `*.d.ts`, and vendored upstream source).
+ * A reference whose target path goes through a `lib/` segment is also skipped:
+ * that is a build OUTPUT (`packages/ui/acp-agent/lib/bin.js`), emitted only by
+ * `pnpm run build`, which CI runs AFTER this gate — flagging it would be a false
+ * positive on a path that is correct but not yet on disk.
  *
  * Run: `tsx scripts/verify-package-paths.ts`.
  */
@@ -111,6 +115,13 @@ function findViolations(absPath: string): Violation[] {
       // class may have swallowed (`packages/core/tools.` / `…/tools/`).
       const ref = m[0].replace(/[./]+$/, '')
       if (existsSync(resolve(root, ref))) continue
+      // A reference INTO a package's built `lib/` is a build-output path, not an
+      // authored-source location: it does not exist until `pnpm run build` emits
+      // it, and CI runs this gate BEFORE the build step. This gate reports stale
+      // SOURCE paths (a moved package), so skip `lib/` targets the same way the
+      // file scan excludes `lib/` files — a `packages/ui/acp-agent/lib/bin.js`
+      // citation in a built-bin smoke is correct, just not yet on disk at lint.
+      if (ref.split('/').includes('lib')) continue
       // Only a stale path to a REAL (moved) package is a violation; a segment
       // matching a live package name is the drift signal.
       const segments = ref.split('/').slice(1)