fix(spec): dist-freshness refusal derives package label and build remedy from pkgDir

.changeset/dist-freshness-refusal-derives-package-label.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/spec': patch
+---
+
+fix(spec): the dist-freshness refusal derives its package label and build remedy from `pkgDir` (#11250)
+
+`inspectDistFreshness()` / `inspectBundleFreshness()` in `packages/spec/scripts/lib/dist-freshness.ts`
+already take an arbitrary `pkgDir`, but their refusal `cause` strings and the `pnpm --filter <pkg> build`
+remedy line hardcoded `packages/spec` / `@objectstack/spec` regardless of it. Every real caller passed
+`SPEC_DIR` until #10969 gave `check:skill-examples` a second surface (`packages/client-react` /
+`packages/client`) — confirmed live: a stale-dist refusal on that surface named `packages/spec` while
+`packages/spec` was freshly built and `client`/`client-react` were the actually-unbuilt packages, so
+following the printed remedy verbatim rebuilt an already-fresh package and re-red identically.
+
+Both cause strings now interpolate a `packages/<name>` label derived from `pkgDir`'s own path (falling
+back to the raw path when the shape doesn't match), and the build-remedy line now reads
+`package.json#name` from `pkgDir` (falling back to the same label when it's missing or unparsable). The
+freshness verdict itself, and every caller's own `rerun` argument, are unchanged — this is diagnostic
+text only.

packages/spec/scripts/dist-freshness.test.ts

@@ -31,7 +31,7 @@ import os from 'node:os';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 
-import { inspectDistFreshness } from './lib/dist-freshness';
+import { inspectDistFreshness, packageDirLabel } from './lib/dist-freshness';
 
 const HERE = path.dirname(fileURLToPath(import.meta.url));
 const PKG = path.resolve(HERE, '..');
@@ -84,7 +84,12 @@ describe('inspectDistFreshness — the dist precondition gen:api-surface never e
  expect(verdict.fresh).toBe(false);
  if (verdict.fresh) return;
  expect(verdict.state).toBe('stale');
- expect(verdict.message).toContain('OLDER than packages/spec/src');
+ // The label is DERIVED from `sandbox`, not a hardcoded `packages/spec` —
+ // `sandbox` is a bare tmpdir with no `packages` path segment, so this
+ // exercises the fallback-to-raw-path branch of `packageDirLabel` (#11250).
+ // The `packages/<name>`-shaped branch is covered by its own case below.
+ expect(verdict.message).toContain(`OLDER than ${packageDirLabel(sandbox)}/src`);
+ expect(verdict.message).not.toContain('packages/spec');
  });
 
  it('refuses in --check mode too, so CI cannot pass against a stale dist either', () => {
@@ -107,6 +112,12 @@ describe('inspectDistFreshness — the dist precondition gen:api-surface never e
  });
 
  it('names the writing damage in generate mode, and prescribes the build', () => {
+ // The build-remedy line now reads `package.json#name` (#11250) rather than
+ // hardcoding `@objectstack/spec`, so THIS case seeds one — matching
+ // `sandbox`'s own docblock, "a throwaway `packages/spec`-SHAPED directory" —
+ // to keep asserting the spec-shaped remedy line. The non-spec-shaped case
+ // below is what proves that name is genuinely read, not assumed.
+ write('package.json', JSON.stringify({ name: '@objectstack/spec' }), NEW);
  write('dist/contracts/index.d.ts', 'export {};', OLD);
  write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW);
 
@@ -118,6 +129,45 @@ describe('inspectDistFreshness — the dist precondition gen:api-surface never e
  expect(verdict.message).toContain('gen:api-surface');
  });
 
+ it('derives BOTH the cause label and the build-remedy package name from a NON-spec-shaped pkgDir (#11250)', () => {
+ // Every case above runs against `sandbox` directly, which (bare tmpdir, no
+ // `packages` path segment) already proves the label is not hardcoded — but
+ // it never exercises the `packages/<name>` MATCHING branch of
+ // `packageDirLabel`, and none of them give `pkgDir` a `package.json` naming
+ // anything other than `@objectstack/spec`. So a build that quietly special-
+ // cased "packages/spec" back in, or that only ever prints the one name
+ // every other case's fixture happens to share, would still pass all of
+ // them. This case is shaped like neither: a real `packages/widgets` path
+ // segment, with its OWN unrelated `package.json#name`.
+ const pkgDir = path.join(sandbox, 'packages', 'widgets');
+ write('packages/widgets/package.json', JSON.stringify({ name: '@acme/widgets' }), NEW);
+ write('packages/widgets/dist/index.d.ts', 'export {};', OLD);
+ write('packages/widgets/src/index.ts', 'export const live = 1;', NEW);
+
+ const verdict = inspectDistFreshness(pkgDir, 'generate', 'pnpm --filter @acme/widgets check:something');
+ expect(verdict.fresh).toBe(false);
+ if (verdict.fresh) return;
+ expect(verdict.state).toBe('stale');
+ expect(verdict.message).toContain('OLDER than packages/widgets/src');
+ expect(verdict.message).toContain('pnpm --filter @acme/widgets build');
+ expect(verdict.message).not.toContain('packages/spec');
+ expect(verdict.message).not.toContain('@objectstack/spec');
+ });
+
+ it('falls back to the raw pkgDir for the build-remedy name when package.json is unreadable', () => {
+ // `packageName`'s OTHER branch (#11250): a `pkgDir` with no `package.json`
+ // at all (or one that fails to parse) must not throw out of a diagnostic
+ // path. `sandbox` itself has none, so this reuses it directly rather than
+ // constructing a second fixture.
+ write('dist/contracts/index.d.ts', 'export {};', OLD);
+ write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW);
+
+ const verdict = inspectDistFreshness(sandbox, 'generate', GEN_RERUN);
+ expect(verdict.fresh).toBe(false);
+ if (verdict.fresh) return;
+ expect(verdict.message).toContain(`pnpm --filter ${packageDirLabel(sandbox)} build`);
+ });
+
  it('reads a MISSING dist as its own condition, not as staleness', () => {
  // A missing dist and a stale one are different facts with different fixes to
  // suggest, and #7122's ruling asked for them to be distinguishable. A never
@@ -130,7 +180,10 @@ describe('inspectDistFreshness — the dist precondition gen:api-surface never e
  expect(verdict.fresh).toBe(false);
  if (verdict.fresh) return;
  expect(verdict.state).toBe('missing');
- expect(verdict.message).toContain('no .d.ts declarations');
+ // Same fallback branch as above: `sandbox` has no `packages` segment, so
+ // the label in the cause string is the raw sandbox path, not `packages/spec`.
+ expect(verdict.message).toContain(`${packageDirLabel(sandbox)}/dist holds no .d.ts declarations`);
+ expect(verdict.message).not.toContain('packages/spec');
  });
 
  it('reads a JS-only dist as missing — the OS_SKIP_DTS=1 shape on a virgin tree', () => {

packages/spec/scripts/lib/dist-freshness.ts

@@ -94,14 +94,68 @@
  * command string rather than an npm script name — that path is not reachable
  * through one.
  */
-import { existsSync, readdirSync } from 'node:fs';
+import { existsSync, readdirSync, readFileSync } from 'node:fs';
 import { join } from 'node:path';
 
 import { bundlesAreStale, distIsStale } from '../../../../scripts/check-regen-pending.mjs';
 
 /** What the generator is about to do, so the refusal can name the real damage. */
 export type DistReadMode = 'generate' | 'check';
 
+/**
+ * A `packages/<name>` display label for `pkgDir`, derived from the path
+ * itself rather than assumed — the refusal text used to hardcode
+ * `packages/spec` unconditionally, which misnamed the stale package for
+ * every non-spec caller (#11250; `check:skill-examples` became the first
+ * real one, adopting this for `packages/client` / `packages/client-react`
+ * in #10969).
+ *
+ * Finds the LAST `packages` path segment and takes the one right after it.
+ * Every real caller today passes an absolute path with a single `packages`
+ * segment, so "last" and "first" coincide; "last" is chosen so a
+ * hypothetical path with an ancestor directory also named `packages`
+ * (`.../packages/foo/packages/bar`) still names the package itself (`bar`)
+ * rather than the misleading earlier match.
+ *
+ * Falls back to the raw `pkgDir` when the shape doesn't match at all (no
+ * `packages` segment, or nothing after it) — a raw path is uglier than a
+ * clean label, but it is still the TRUE location, whereas a hardcoded
+ * `packages/spec` from a `pkgDir` this rule failed to parse would be a wrong
+ * one dressed as a right one.
+ */
+export function packageDirLabel(pkgDir: string): string {
+ const segments = pkgDir.split(/[\\/]+/).filter(Boolean);
+ const idx = segments.lastIndexOf('packages');
+ if (idx === -1 || idx === segments.length - 1) return pkgDir;
+ return `packages/${segments[idx + 1]}`;
+}
+
+/**
+ * The package's own declared `package.json#name` — what `pnpm --filter`
+ * actually resolves against, which is why the build-remedy line needs THIS
+ * and not `packageDirLabel` above (a directory label like `packages/client`
+ * is not a valid `--filter` argument; the published name is
+ * `@objectstack/client`).
+ *
+ * Read defensively: a missing or unparsable `package.json` falls back to the
+ * directory label. That fallback command will not resolve either, but it is
+ * no worse than the pre-fix behaviour (which unconditionally printed
+ * `@objectstack/spec` regardless of `pkgDir`) and it stays legible rather
+ * than throwing out of a diagnostic path.
+ */
+function packageName(pkgDir: string): string {
+ try {
+ const parsed = JSON.parse(readFileSync(join(pkgDir, 'package.json'), 'utf8')) as {
+ name?: unknown;
+ };
+ if (typeof parsed.name === 'string' && parsed.name.length > 0) return parsed.name;
+ } catch {
+ // Falls through to the directory-label fallback below — unreadable or
+ // unparsable package.json is not fatal to a diagnostic message.
+ }
+ return packageDirLabel(pkgDir);
+}
+
 export type DistFreshness =
  | { fresh: true }
  | { fresh: false; state: 'missing' | 'stale'; message: string };
@@ -164,11 +218,12 @@ export function inspectDistFreshness(
  ` check would reach its conclusion without ever reading the declarations under test —\n` +
  ` a FALSE GREEN on exactly the change it exists to catch (#7122).`;
 
+ const label = packageDirLabel(pkgDir);
  const cause =
  state === 'missing'
- ? `packages/spec/dist holds no .d.ts declarations — the package is not built (or was built\n` +
+ ? `${label}/dist holds no .d.ts declarations — the package is not built (or was built\n` +
  ` with OS_SKIP_DTS=1, which emits JS and skips exactly the artifact this reads).`
- : `packages/spec/dist/**/*.d.ts is OLDER than packages/spec/src — the declarations on disk\n` +
+ : `${label}/dist/**/*.d.ts is OLDER than ${label}/src — the declarations on disk\n` +
  ` predate the sources. If you built with OS_SKIP_DTS=1, that build did not rebuild them.`;
 
  return {
@@ -178,7 +233,7 @@ export function inspectDistFreshness(
  `\n❌ ${cause}\n\n` +
  ` ${damage}\n\n` +
  ` Build first, then re-run:\n\n` +
- ` pnpm --filter @objectstack/spec build\n` +
+ ` pnpm --filter ${packageName(pkgDir)} build\n` +
  ` ${rerun}\n\n` +
  ` (Do NOT use OS_SKIP_DTS=1 for this one — AGENTS.md §9 names it as the flag that emits JS\n` +
  ` and skips exactly the declarations this reads.)`,
@@ -239,11 +294,12 @@ export function inspectBundleFreshness(
  ` check would reach its conclusion without ever reading the module graph under test —\n` +
  ` NOT MEASURED reported as if it were measured and clean (#4690).`;
 
+ const label = packageDirLabel(pkgDir);
  const cause =
  state === 'missing'
- ? `packages/spec/dist holds no .mjs/.js bundles — the package is not built. This gate reads\n` +
+ ? `${label}/dist holds no .mjs/.js bundles — the package is not built. This gate reads\n` +
  ` the module a consumer's import actually loads, so there is nothing here to read.`
- : `packages/spec/dist's .mjs/.js bundles are OLDER than packages/spec/src (or than\n` +
+ : `${label}/dist's .mjs/.js bundles are OLDER than ${label}/src (or than\n` +
  ` tsup.config.ts, which decides the entries, the externals and whether entries are\n` +
  ` self-contained). The bundles on disk predate the sources.`;
 
@@ -254,7 +310,7 @@ export function inspectBundleFreshness(
  `\n❌ ${cause}\n\n` +
  ` ${damage}\n\n` +
  ` Build first, then re-run:\n\n` +
- ` pnpm --filter @objectstack/spec build\n` +
+ ` pnpm --filter ${packageName(pkgDir)} build\n` +
  ` ${rerun}\n\n` +
  ` (OS_SKIP_DTS=1 is fine for THIS gate — it still emits every bundle this reads. It is\n` +
  ` the .d.ts-reading gates next door that it blinds.)`,