diff --git a/.changeset/data-objectstack-chatbot-list-spec-symbol-burn-down.md b/.changeset/data-objectstack-chatbot-list-spec-symbol-burn-down.md new file mode 100644 index 0000000000..b2c6a05e88 --- /dev/null +++ b/.changeset/data-objectstack-chatbot-list-spec-symbol-burn-down.md @@ -0,0 +1,60 @@ +--- +"@object-ui/data-objectstack": major +"@object-ui/plugin-chatbot": major +"@object-ui/plugin-list": major +--- + +Stop declaring 12 `@object-ui/data-objectstack` / `@object-ui/plugin-chatbot` / +`@object-ui/plugin-list` symbols under names `@objectstack/spec` owns +(objectui#3160, objectstack#4115 batch 6). All three packages leave the ledger. + +**Breaking for importers of `@object-ui/data-objectstack`** — four exported +names changed, because the spec exports the same name for a *different* thing: + +| was | now | what the spec's same-named export actually is | +|:--|:--|:--| +| `CacheStats` | `MetadataCacheStats` | the platform `ICacheService` counters (`keyCount`, `memoryUsage`) | +| `MetadataSaveOptions` | `MetadataClientSaveOptions` | options for writing a metadata item to a **file** (`format`, `path`, `indent`, `atomic`) | +| `SecurityPolicy` | `SecurityManagerPolicy` | the package supply-chain policy (`autoScan`, licences, code signing, sandbox) | +| `ValidationError` | `DataApiValidationError` | a plain `{ field, message, code? }` entry in a validation report | + +Each pair is disjoint or nearly so — `MetadataSaveOptions` and `SecurityPolicy` +share not one key with the spec type whose name they wore — so none of them was +a dialect to reconcile; they were four unrelated concepts squatting on spec +names. `DataApiValidationError` follows the `Validation` +convention registered on objectstack#4115 (`@object-ui/core` took +`SchemaNodeValidationError` in batch 4). Its **runtime** `name` deliberately +stays `'ValidationError'`: `normaliseClientError` and `@object-ui/react`'s +error-message helper both sniff `err.name`, so that string is a wire contract, +not a symbol. + +**Breaking for importers of `@object-ui/plugin-chatbot`** — `PendingActionRow` +and `PendingActionStatus` are now re-exported from `@objectstack/spec/contracts` +instead of hand-transcribed, which narrows them. The copies had drifted three +ways, and each drift had **disabled a compile-time check** rather than merely +differed from one: + +- `status: PendingActionStatus | string` — a union with `string` absorbs the + literals, so that annotation carried no information at all; +- `[key: string]: unknown` — the objectstack#4075 mechanism: an index signature + makes every structural comparison against the spec answer "identical", however + far the copy has drifted; +- `created_at` / `updated_at`, which the service contract does not carry and no + consumer in this repo reads. + +**Breaking for importers of `@object-ui/plugin-list`** — `ViewTab` is derived from the spec's `ViewTabSchema` +— from its **input** side, because `pinned` / `isDefault` / `visible` carry +`.default()`s and this component is handed authored metadata, not parsed output. +That removes a renderer-side tolerance the copy carried: `visible` accepted +`string | boolean` and the tab bar compared it against the literal `'false'`, a +spelling no producer emits. `label` also stops being required (the spec makes it +optional; `name` is the identifier) and `filter` stops being `any`. + +`ListView` and `UserFilters` keep their names as declared dialects: both are the +React **renderers** of the spec types whose names they share, and each takes that +spec type as a prop (`ListViewProps.schema`, `UserFiltersProps.config`) rather +than restating its shape. `Tool` and `MessageContent` in `plugin-chatbot` are +vendored Vercel AI Elements / Shadcn primitives — upstream's component API, not +objectui's authored surface — so the guard now skips that directory the same way +it already skips `components/src/ui/`, with a test that fails if any file there +stops carrying its vendor banner. diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index b6ce20046a..423294487a 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -179,7 +179,12 @@ import { BulkOperationError, // Bulk operation failures with partial results ConnectionError, // Network/connection errors (503/504) AuthenticationError, // Authentication failures (401/403) - ValidationError, // Data validation errors (400) + DataApiValidationError, // Data validation errors (400). Its runtime `name` is + // still 'ValidationError' — that string is the wire + // discriminator shared with @objectstack/client. The + // SYMBOL is prefixed because @objectstack/spec/kernel + // owns `ValidationError` for a { field, message, code? } + // record (objectui#3160). } from '@object-ui/data-objectstack'; ``` diff --git a/packages/data-objectstack/package.json b/packages/data-objectstack/package.json index 73a4783a4a..4d0eebfedf 100644 --- a/packages/data-objectstack/package.json +++ b/packages/data-objectstack/package.json @@ -32,10 +32,10 @@ "dependencies": { "@object-ui/core": "workspace:*", "@object-ui/types": "workspace:*", - "@objectstack/client": "^17.0.0-rc.1" + "@objectstack/client": "^17.0.0-rc.1", + "@objectstack/spec": "^17.0.0-rc.1" }, "devDependencies": { - "@objectstack/spec": "^17.0.0-rc.1", "tsup": "^8.5.1", "typescript": "^6.0.3", "vitest": "^4.1.10" diff --git a/packages/data-objectstack/src/cache/MetadataCache.ts b/packages/data-objectstack/src/cache/MetadataCache.ts index 28ea62e225..065238a7ea 100644 --- a/packages/data-objectstack/src/cache/MetadataCache.ts +++ b/packages/data-objectstack/src/cache/MetadataCache.ts @@ -17,9 +17,18 @@ interface CachedSchema { } /** - * Cache statistics for monitoring + * Statistics reported by {@link MetadataCache.getStats} for monitoring. + * + * NOT the spec's `CacheStats` (`@objectstack/spec/contracts`), whose name this + * interface wore until objectui#3160 (objectstack#4115 ledger batch 6). That one + * describes the platform's `ICacheService` — a server-side KV cache measured by + * `keyCount` and `memoryUsage`. This one describes the browser-side LRU in front + * of `/api/v1/meta/*`: it is bounded (`size`/`maxSize`), it evicts, it coalesces + * concurrent fetches onto one in-flight promise, and it reports a `hitRate`. + * Neither type has a key the other has, so this is a name collision, not a + * dialect — deriving would have replaced every field. */ -export interface CacheStats { +export interface MetadataCacheStats { size: number; maxSize: number; hits: number; @@ -218,7 +227,7 @@ export class MetadataCache { * * @returns Cache statistics including hit rate */ - getStats(): CacheStats { + getStats(): MetadataCacheStats { const total = this.stats.hits + this.stats.misses; const hitRate = total > 0 ? this.stats.hits / total : 0; diff --git a/packages/data-objectstack/src/errors.test.ts b/packages/data-objectstack/src/errors.test.ts index 80acfa12f2..1f8a37c2a2 100644 --- a/packages/data-objectstack/src/errors.test.ts +++ b/packages/data-objectstack/src/errors.test.ts @@ -13,7 +13,7 @@ import { BulkOperationError, ConnectionError, AuthenticationError, - ValidationError, + DataApiValidationError, createErrorFromResponse, isObjectStackError, isErrorType, @@ -189,20 +189,20 @@ describe('Error Classes', () => { }); }); - describe('ValidationError', () => { + describe('DataApiValidationError', () => { it('should create validation error', () => { - const error = new ValidationError('Invalid input'); + const error = new DataApiValidationError('Invalid input'); expect(error.message).toBe('Invalid input'); expect(error.code).toBe('VALIDATION_ERROR'); expect(error.statusCode).toBe(400); expect(error.name).toBe('ValidationError'); expect(error).toBeInstanceOf(ObjectStackError); - expect(error).toBeInstanceOf(ValidationError); + expect(error).toBeInstanceOf(DataApiValidationError); }); it('should include field information', () => { - const error = new ValidationError('Email is invalid', 'email'); + const error = new DataApiValidationError('Email is invalid', 'email'); expect(error.field).toBe('email'); expect(error.details).toHaveProperty('field', 'email'); @@ -214,7 +214,7 @@ describe('Error Classes', () => { { field: 'age', message: 'Must be a positive number' }, ]; - const error = new ValidationError( + const error = new DataApiValidationError( 'Validation failed', undefined, validationErrors @@ -225,7 +225,7 @@ describe('Error Classes', () => { }); it('should return empty array when no validation errors', () => { - const error = new ValidationError('Validation failed'); + const error = new DataApiValidationError('Validation failed'); expect(error.getValidationErrors()).toEqual([]); }); @@ -286,7 +286,7 @@ describe('Error Helpers', () => { expect(error.code).toBe('NOT_FOUND'); }); - it('should create ValidationError for 400 status', () => { + it('should create DataApiValidationError for 400 status', () => { const response = { status: 400, message: 'Bad request', @@ -299,9 +299,9 @@ describe('Error Helpers', () => { const error = createErrorFromResponse(response); - expect(error).toBeInstanceOf(ValidationError); + expect(error).toBeInstanceOf(DataApiValidationError); expect(error.statusCode).toBe(400); - expect((error as ValidationError).validationErrors).toEqual([ + expect((error as DataApiValidationError).validationErrors).toEqual([ { field: 'email', message: 'Invalid email' }, ]); }); @@ -379,7 +379,7 @@ describe('Error Helpers', () => { const bulkError = new BulkOperationError('create', 0, 1, []); const connError = new ConnectionError('timeout'); const authError = new AuthenticationError(); - const validError = new ValidationError('invalid'); + const validError = new DataApiValidationError('invalid'); expect(isObjectStackError(metadataError)).toBe(true); expect(isObjectStackError(bulkError)).toBe(true); @@ -409,7 +409,7 @@ describe('Error Helpers', () => { it('should return false for non-matching error type', () => { const error = new MetadataNotFoundError('users'); - expect(isErrorType(error, ValidationError)).toBe(false); + expect(isErrorType(error, DataApiValidationError)).toBe(false); }); it('should return true for base class check', () => { diff --git a/packages/data-objectstack/src/errors.ts b/packages/data-objectstack/src/errors.ts index 83e3daf456..89faa187de 100644 --- a/packages/data-objectstack/src/errors.ts +++ b/packages/data-objectstack/src/errors.ts @@ -159,12 +159,24 @@ export class AuthenticationError extends ObjectStackError { } /** - * Error thrown when data validation fails + * Error thrown when the ObjectStack data API rejects a write as invalid. + * + * NOT the spec's `ValidationError` (`@objectstack/spec/kernel`), whose name this + * class wore until objectui#3160 (objectstack#4115 ledger batch 6). That one is + * a plain DATA SHAPE — `{ field, message, code? }`, one entry in a plugin + * manifest's validation report — and `@object-ui/types` re-exports it under that + * name. This is a runtime `Error` subclass carrying an HTTP status plus a list + * of such entries, so the two are not even the same KIND of thing. + * + * The name follows the convention registered on objectstack#4115 for this + * family — `Validation`. `@object-ui/core` + * took `SchemaNodeValidationError` for its SDUI-tree walk; this one belongs to + * the data API. */ -export class ValidationError extends ObjectStackError { +export class DataApiValidationError extends ObjectStackError { /** - * Create a new ValidationError - * + * Create a new DataApiValidationError + * * @param message - Human-readable error message * @param field - The field that failed validation (optional) * @param validationErrors - Array of validation error details @@ -186,6 +198,12 @@ export class ValidationError extends ObjectStackError { ...details, } ); + // The RUNTIME name stays `'ValidationError'` on purpose — it is the wire + // discriminator this adapter shares with `@objectstack/client` and with + // consumers that sniff `err.name` rather than `instanceof` + // (`normaliseClientError` here, `@object-ui/react`'s `error-message`). + // Renaming the TypeScript symbol is a source-level rename; renaming this + // string would be a behaviour change nobody asked for. this.name = 'ValidationError'; } @@ -232,7 +250,7 @@ export function createErrorFromResponse(response: Record, conte return new ObjectStackError(message, 'NOT_FOUND', 404, details); case 400: - return new ValidationError(message, undefined, (response?.data as Record)?.errors as Array<{ field: string; message: string }>, details); + return new DataApiValidationError(message, undefined, (response?.data as Record)?.errors as Array<{ field: string; message: string }>, details); case 503: return new ConnectionError(message, (response?.config as Record)?.url as string, details, 503); diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index af80e77f7f..3b49f355e0 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -7,6 +7,7 @@ */ import { ObjectStackClient, type QueryOptions as ObjectStackQueryOptions } from '@objectstack/client'; +import type { DroppedFieldsEvent } from '@objectstack/spec/data'; import type { DataSource, BatchTransactionOperation, @@ -39,7 +40,7 @@ import { MetadataNotFoundError, BulkOperationError, ConnectionError, - ValidationError, + DataApiValidationError, createErrorFromResponse, } from './errors'; @@ -588,7 +589,7 @@ export function isConcurrentUpdateError(error: unknown): error is ConcurrentUpda * * Two shapes are recognised: * - `409` + `CONCURRENT_UPDATE` → {@link ConcurrentUpdateError}; - * - `400` + `VALIDATION_FAILED` → {@link ValidationError}, carrying the + * - `400` + `VALIDATION_FAILED` → {@link DataApiValidationError}, carrying the * server's per-field entries so a form can mark the offending inputs * instead of showing one undirected toast. */ @@ -621,7 +622,7 @@ export function normaliseClientError(error: unknown): unknown { }) .filter((x): x is { field: string; message: string } => x !== null); - return new ValidationError( + return new DataApiValidationError( typeof e.message === 'string' ? e.message : 'Validation failed', validationErrors[0]?.field, validationErrors, @@ -721,16 +722,21 @@ export type BatchProgressListener = (event: BatchProgressEvent) => void; /** * One server-reported write-strip: caller-supplied fields the backend LEGALLY * removed from a write before persisting (a non-system caller cannot seed a - * `readonly` field, a `readonlyWhen` predicate locked it, etc.). Mirrors the - * framework `DroppedFieldsEvent` (spec `DroppedFieldsEventSchema`) structurally - * so we don't pin a client type version — `reason` is kept as a widened string - * for forward-compatibility with reasons added server-side. + * `readonly` field, a `readonlyWhen` predicate locked it, etc.). + * + * THE spec type, re-exported (objectui#3160, objectstack#4115 ledger batch 6). + * Until then this was a hand copy whose comment said it "mirrors the framework + * `DroppedFieldsEvent` (spec `DroppedFieldsEventSchema`) structurally so we + * don't pin a client type version", with `reason` widened from the spec's + * `'readonly' | 'readonly_when'` to bare `string` "for forward-compatibility + * with reasons added server-side". Both halves of that reasoning are the + * failure mode this ledger exists to remove: the spec IS the client type + * version, and a consumer-side widening of a producer's enum is precisely the + * lenient fallback AGENTS.md #12 bans — it deletes the only compile-time signal + * that would tell `AdapterProvider`'s toast wording (which branches on + * `readonly_when`) that a new reason had appeared. */ -export interface DroppedFieldsEvent { - object: string; - fields: string[]; - reason: string; -} +export type { DroppedFieldsEvent }; /** * Emitted after a create/update whose response carried `droppedFields` @@ -3511,14 +3517,14 @@ export { BulkOperationError, ConnectionError, AuthenticationError, - ValidationError, + DataApiValidationError, createErrorFromResponse, isObjectStackError, isErrorType, } from './errors'; // Export cache types -export type { CacheStats } from './cache/MetadataCache'; +export type { MetadataCacheStats } from './cache/MetadataCache'; // v3.0.0 Deep Integration modules export { CloudOperations } from './cloud'; @@ -3546,7 +3552,7 @@ export type { MetadataClientConfig, MetadataListOptions, MetadataDraftHeader, - MetadataSaveOptions, + MetadataClientSaveOptions, MetadataGetOptions, MetadataDeleteOptions, MetadataHistoryOptions, @@ -3563,7 +3569,7 @@ export type { } from './metadata-client'; export { SecurityManager } from './security'; -export type { SecurityPolicy, CSPConfig, AuditLogConfig, AuditEventType, DataMaskingConfig, DataMaskingRule, AuditLogEntry } from './security'; +export type { SecurityManagerPolicy, CSPConfig, AuditLogConfig, AuditEventType, DataMaskingConfig, DataMaskingRule, AuditLogEntry } from './security'; export { createDefaultCanvasConfig, snapToGrid, calculateAutoLayout } from './studio'; export type { StudioCanvasConfig, StudioPropertyEditor, StudioThemeBuilderConfig, StudioColorPalette, StudioTypographyPreset, StudioShadowPreset } from './studio'; diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index d33145a57b..de80ce89b7 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -75,7 +75,21 @@ export interface MetadataDraftHeader { updatedBy: string | null; } -export interface MetadataSaveOptions { +/** + * Options for {@link MetadataClient.save} — a WRITE OVER HTTP to + * `/api/v1/meta/:type/:name`. + * + * NOT the spec's `MetadataSaveOptions` (`@objectstack/spec/system` and + * `/kernel`), whose name this interface wore until objectui#3160 + * (objectstack#4115 ledger batch 6). Both spec copies describe writing a + * metadata item to a FILE — `format: json|yaml|ts`, `path`, `indent`, + * `prettify`, `sortKeys`, `backup`, `atomic`, `loader`. Not one of those keys + * exists here, and not one of these exists there: this is the REST client's + * request envelope — optimistic concurrency (`ifMatch` → `If-Match`), actor + * attribution, the destructive-change override, the ADR-0033 draft/publish + * mode, and the owning package. Same words, different layer. + */ +export interface MetadataClientSaveOptions { /** * Optimistic concurrency token (the `checksum` returned by the last * read). When present, sent as the `If-Match` header so concurrent @@ -122,7 +136,7 @@ export interface MetadataGetOptions { packageId?: string; } -export interface MetadataDeleteOptions extends MetadataSaveOptions { +export interface MetadataDeleteOptions extends MetadataClientSaveOptions { /** * Target state. `'draft'` discards the pending draft (keeps the * published overlay intact). Omit to reset the active overlay back @@ -527,7 +541,7 @@ export class MetadataClient { type: string, name: string, item: unknown, - options: MetadataSaveOptions = {}, + options: MetadataClientSaveOptions = {}, ): Promise { if (!name || !String(name).trim()) { // The `PUT /meta/:type/:name` route rejects a missing `:name` segment diff --git a/packages/data-objectstack/src/security.ts b/packages/data-objectstack/src/security.ts index 1994a617f5..7ba8288c02 100644 --- a/packages/data-objectstack/src/security.ts +++ b/packages/data-objectstack/src/security.ts @@ -11,7 +11,20 @@ * Provides advanced security policies: CSP config, audit logging, data masking. */ -export interface SecurityPolicy { +/** + * Configuration for {@link SecurityManager} — this adapter's browser-side + * security posture: the CSP header it generates, the in-memory audit log it + * keeps, and the field-masking rules it applies before a record is rendered. + * + * NOT the spec's `SecurityPolicy` (`@objectstack/spec/kernel`), whose name this + * interface wore until objectui#3160 (objectstack#4115 ledger batch 6). That one + * is the PACKAGE SUPPLY-CHAIN policy — `{ id, name, autoScan, thresholds, + * allowedLicenses, prohibitedLicenses, codeSigning, sandbox }` — the rules a + * marketplace applies when scanning a plugin before install. It shares no key + * with this one, so there was nothing to derive; the collision is the word + * "security", not the concept. + */ +export interface SecurityManagerPolicy { /** Content Security Policy configuration */ csp?: CSPConfig; /** Audit logging configuration */ @@ -100,10 +113,10 @@ export interface AuditLogEntry { * Handles CSP generation, audit logging, and data masking. */ export class SecurityManager { - private policy: SecurityPolicy; + private policy: SecurityManagerPolicy; private auditLog: AuditLogEntry[] = []; - constructor(policy: SecurityPolicy = {}) { + constructor(policy: SecurityManagerPolicy = {}) { this.policy = policy; } @@ -204,14 +217,14 @@ export class SecurityManager { /** * Update the security policy. */ - updatePolicy(policy: Partial): void { + updatePolicy(policy: Partial): void { this.policy = { ...this.policy, ...policy }; } /** * Get current security policy. */ - getPolicy(): SecurityPolicy { + getPolicy(): SecurityManagerPolicy { return { ...this.policy }; } } diff --git a/packages/data-objectstack/src/spec-symbol-batch6.test.ts b/packages/data-objectstack/src/spec-symbol-batch6.test.ts new file mode 100644 index 0000000000..d31ed51ae0 --- /dev/null +++ b/packages/data-objectstack/src/spec-symbol-batch6.test.ts @@ -0,0 +1,303 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `@object-ui/data-objectstack` ↔ `@objectstack/spec` symbol-collision guards + * (objectui#3160, objectstack#4115 ledger batch 6). + * + * Five symbols here wore names the spec already owns. One really WAS the spec's + * and now carries its binding (`DroppedFieldsEvent`); the other four model + * something the spec has never modelled under that name and were renamed: + * + * CacheStats → MetadataCacheStats + * MetadataSaveOptions → MetadataClientSaveOptions + * SecurityPolicy → SecurityManagerPolicy + * ValidationError → DataApiValidationError + * + * The batch's own triage note said "data-objectstack is a direct client of the + * spec protocol, so `SecurityPolicy` / `DroppedFieldsEvent` are probably hand + * copies — derive them first". Half of that held: `DroppedFieldsEvent` was + * exactly a hand copy, and `SecurityPolicy` turned out to share not one key with + * the spec's. That asymmetry is why every assertion below is per SYMBOL and + * never per cluster. + * + * ## Why the spec's names are read through the compiler, not `import * as` + * + * A runtime namespace import sees VALUES only, and every name checked here is a + * TYPE; a tripwire built on `Object.keys(await import(…))` would pass while + * proving nothing. So this reads each subpath's `.d.ts` through the TypeScript + * checker, exactly as `scripts/check-spec-symbol-derivation.mjs` does. + * + * ## …and why through `ts.sys` rather than `node:fs` + * + * Unlike the sibling batches' parity tests, this package's `tsconfig.json` + * compiles its whole test tree (it is not in `check-type-check-coverage.mjs`'s + * TEST_DEBT), so the pins below are already checked by `tsc --noEmit` with no + * separate `tsconfig.typetests.json`. Keeping that property means NOT pulling + * `@types/node` into a browser-side adapter's type environment just to read a + * file — so the probe uses the TypeScript compiler's own host, which is typed + * by the `typescript` dependency this file already has, and resolves the spec + * through TS's module resolver instead of hard-coding its subpath list. + */ + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; + +import { DataApiValidationError } from './errors'; +import type { MetadataCacheStats } from './cache/MetadataCache'; +import type { MetadataClientSaveOptions } from './metadata-client'; +import type { SecurityManagerPolicy } from './security'; +import type { DroppedFieldsEvent } from './index'; + +import type { DroppedFieldsEvent as SpecDroppedFieldsEvent } from '@objectstack/spec/data'; +import type { CacheStats as SpecCacheStats } from '@objectstack/spec/contracts'; +import type { + MetadataSaveOptions as SpecMetadataSaveOptions, + SecurityPolicy as SpecSecurityPolicy, + ValidationError as SpecValidationError, +} from '@objectstack/spec/kernel'; + +const PROBE_OPTIONS: ts.CompilerOptions = { + noEmit: true, + skipLibCheck: true, + strict: false, + target: ts.ScriptTarget.ESNext, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + resolveJsonModule: true, +}; + +/** This file, as TypeScript's module resolver wants it: an absolute path. */ +const HERE = new URL(import.meta.url).pathname; + +/** Every name `@objectstack/spec` exports from any subpath — types AND values. */ +function specExportNames(): Set { + const pkgPath = ts.resolveModuleName('@objectstack/spec/package.json', HERE, PROBE_OPTIONS, ts.sys) + .resolvedModule?.resolvedFileName; + if (!pkgPath) { + throw new Error('cannot resolve @objectstack/spec — run `pnpm install` first'); + } + const pkgDir = pkgPath.slice(0, pkgPath.lastIndexOf('/')); + const raw = ts.sys.readFile(pkgPath); + if (!raw) throw new Error(`cannot read ${pkgPath}`); + const pkg = JSON.parse(raw) as { + exports?: Record; + }; + + // Read the export map rather than a hand-written subpath list: a list is a + // copy, and a copy of the spec's shape is what this whole ledger is about. + const files: string[] = []; + for (const cond of Object.values(pkg.exports ?? {})) { + if (typeof cond !== 'object' || cond === null) continue; + const dts = cond.import?.types ?? cond.require?.types; + if (dts) files.push(ts.sys.resolvePath(`${pkgDir}/${dts.replace(/^\.\//, '')}`)); + } + + const program = ts.createProgram(files, PROBE_OPTIONS); + const checker = program.getTypeChecker(); + + const names = new Set(); + for (const file of files) { + const sf = program.getSourceFile(file); + if (!sf) continue; + const moduleSymbol = checker.getSymbolAtLocation(sf); + if (!moduleSymbol) continue; + for (const exported of checker.getExportsOfModule(moduleSymbol)) names.add(exported.getName()); + } + return names; +} + +const SPEC_NAMES = specExportNames(); + +describe('the spec export-name probe itself works', () => { + it('reads a non-trivial number of names', () => { + expect(SPEC_NAMES.size).toBeGreaterThan(1000); + }); + + it('sees TYPE-only exports, not just runtime values', () => { + // `DroppedFieldsEvent` is a type alias — invisible to a runtime `import()`. + expect(SPEC_NAMES.has('DroppedFieldsEvent')).toBe(true); + }); +}); + +const RENAMES: Array<[local: string, formerly: string, specMeaning: string]> = [ + [ + 'MetadataCacheStats', + 'CacheStats', + "the platform's ICacheService counters (keyCount / memoryUsage)", + ], + [ + 'MetadataClientSaveOptions', + 'MetadataSaveOptions', + 'options for writing a metadata item to a FILE (format / path / indent / atomic)', + ], + [ + 'SecurityManagerPolicy', + 'SecurityPolicy', + 'the package supply-chain policy (autoScan / licences / codeSigning / sandbox)', + ], + [ + 'DataApiValidationError', + 'ValidationError', + 'a plain { field, message, code? } entry in a validation report', + ], +]; + +describe('renamed local concepts do not collide with a spec export', () => { + it.each(RENAMES)('the spec does not own `%s`', (local) => { + expect( + SPEC_NAMES.has(local), + `@objectstack/spec now exports \`${local}\`. This package declares its own ` + + `\`${local}\`, so the rename that fixed objectstack#4115 has re-created the ` + + `collision under the new name. Rename again — and check the new name here ` + + `FIRST: objectui#3074 landed a rename straight onto another spec export, and ` + + `batch 4 burned two obvious candidates (SchemaValidationResult, ` + + `SchemaValidationReport) the same way.`, + ).toBe(false); + }); + + it.each(RENAMES)('the spec still owns `%s` (it means: %s)', (_local, formerly) => { + expect( + SPEC_NAMES.has(formerly), + `@objectstack/spec no longer exports \`${formerly}\`, which is the only reason ` + + `this package renamed it. Either the spec dropped the name (then the plain ` + + `name can be taken back) or it moved (then re-read what it means now). A ` + + `workaround must not outlive its reason.`, + ).toBe(true); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Compile-time pins. A violation is a `tsc` error, not a runtime failure. */ +/* Compiled by this package's own `tsc --noEmit`: unlike its siblings, this */ +/* package's tsconfig.json does NOT exclude test files, so no separate */ +/* tsconfig.typetests.json is needed to make these pins load-bearing. */ +/* -------------------------------------------------------------------------- */ + +type Assert = T; +type IsAny = 0 extends 1 & T ? true : false; +/** The `unknown` erasure the `any` probe reports `false` for (objectui#3155). */ +type IsUnknown = [unknown] extends [T] ? ([T] extends [unknown] ? true : false) : false; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; +type HasKey = K extends keyof T ? true : false; + +describe('DroppedFieldsEvent IS the spec type, not a mirror of it', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + type _SpecNotUnknown = Assert, false>>; + + type _IsTheSpecType = Assert>; + + // The pin that matters. The deleted copy widened `reason` to bare `string` + // "for forward-compatibility with reasons added server-side" — which is the + // AGENTS.md #12 lenient fallback, and it deleted the only signal that would + // tell AdapterProvider's toast (it branches on `readonly_when`) that a third + // reason had appeared. `Equal` is used rather than `extends` because + // `'readonly' | 'readonly_when' extends string` is true in the drifted + // direction too. + type _ReasonIsTheEnum = Assert>; + type _ReasonIsNotString = Assert, false>>; + + expect(true).toBe(true); + }); +}); + +describe('MetadataCacheStats is not the spec ICacheService stats', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + + // The two agree only on the two counters every cache has. Everything that + // says what KIND of cache each describes is exclusive to one side: the + // spec's is a server KV store sized in keys and bytes; this one is a bounded + // browser LRU that evicts, coalesces in-flight fetches and reports a rate. + type _Shared = Assert, 'hits' | 'misses'>>; + type _SpecOnly = Assert, 'keyCount' | 'memoryUsage'>>; + type _LocalOnly = Assert< + Equal< + Exclude, + 'size' | 'maxSize' | 'evictions' | 'coalesced' | 'hitRate' + > + >; + + expect(true).toBe(true); + }); +}); + +describe('MetadataClientSaveOptions is not the spec file-save options', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + + // Disjoint: the spec's describes serialising an item to disk, this one + // describes a PUT to /api/v1/meta/*. Not one key is shared, so there was + // never a subset relationship to derive from. + type _NoOverlap = Assert, never>>; + + // The keys that make this the HTTP envelope. + type _HasIfMatch = Assert>; + type _HasDraftMode = Assert>; + // …and the ones that make the spec's the file writer. + type _SpecHasPath = Assert>; + type _SpecHasFormat = Assert>; + + expect(true).toBe(true); + }); +}); + +describe('SecurityManagerPolicy is not the spec package-security policy', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + + // Also fully disjoint. The spec's governs installing a plugin (scan + // schedule, licence allowlists, code signing, sandbox limits); this one + // governs what the browser adapter emits and hides (CSP header, audit log, + // field masking). + type _NoOverlap = Assert, never>>; + type _SpecIsIdentified = Assert>; + type _LocalIsAllOptional = Assert< + Equal< + undefined extends SecurityManagerPolicy['csp' | 'auditLog' | 'dataMasking'] ? true : false, + true + > + >; + + expect(true).toBe(true); + }); +}); + +describe('DataApiValidationError is an Error, the spec ValidationError is a record', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + + // The spec's is one entry in a validation report. `@object-ui/types` + // re-exports it under that name, so the two were importable side by side. + type _SpecShape = Assert, never>>; + + // This one is a thrown runtime object: it has a `stack`, which no data + // record does, and it carries the whole list the spec type is one item of. + type _WeAreAnError = Assert>; + type _WeCarryTheList = Assert< + Equal | undefined> + >; + + expect(true).toBe(true); + }); + + it('keeps `name === "ValidationError"` — the RUNTIME name is a wire contract', () => { + // Deliberately NOT renamed with the class. `normaliseClientError` in + // ./index.ts sniffs `e.name === 'ValidationError'` on errors thrown by + // `@objectstack/client`, and `@object-ui/react`'s `error-message` does the + // same on the way out. Renaming the TypeScript symbol is a source change; + // renaming this string would be a silent behaviour change. + expect(new DataApiValidationError('nope').name).toBe('ValidationError'); + expect(new DataApiValidationError('nope')).toBeInstanceOf(Error); + expect(new DataApiValidationError('nope').code).toBe('VALIDATION_ERROR'); + expect(new DataApiValidationError('nope').statusCode).toBe(400); + }); +}); diff --git a/packages/data-objectstack/src/validation-error.test.ts b/packages/data-objectstack/src/validation-error.test.ts index 078cc9571c..118f0319d3 100644 --- a/packages/data-objectstack/src/validation-error.test.ts +++ b/packages/data-objectstack/src/validation-error.test.ts @@ -9,7 +9,7 @@ import { describe, it, expect } from 'vitest'; import { normaliseClientError } from './index'; -import { ValidationError } from './errors'; +import { DataApiValidationError } from './errors'; /** * The server has always sent per-field rejection detail; the adapter used to @@ -35,13 +35,13 @@ describe('normaliseClientError — VALIDATION_FAILED', () => { }, }); - it('wraps it into a typed ValidationError', () => { + it('wraps it into a typed DataApiValidationError', () => { const normalised = normaliseClientError(upstream()); - expect(normalised).toBeInstanceOf(ValidationError); + expect(normalised).toBeInstanceOf(DataApiValidationError); }); it('carries every field entry through', () => { - const normalised = normaliseClientError(upstream()) as ValidationError; + const normalised = normaliseClientError(upstream()) as DataApiValidationError; expect(normalised.validationErrors).toEqual([ { field: 'name', message: 'Name is required' }, { field: 'stage', message: 'Stage is not a valid option' }, @@ -49,12 +49,12 @@ describe('normaliseClientError — VALIDATION_FAILED', () => { }); it('keeps the server message verbatim — it names every field', () => { - const normalised = normaliseClientError(upstream()) as ValidationError; + const normalised = normaliseClientError(upstream()) as DataApiValidationError; expect(normalised.message).toBe('Name is required; Stage is not a valid option'); }); it('exposes the first offending field on `.field`', () => { - const normalised = normaliseClientError(upstream()) as ValidationError; + const normalised = normaliseClientError(upstream()) as DataApiValidationError; expect(normalised.field).toBe('name'); }); @@ -63,7 +63,7 @@ describe('normaliseClientError — VALIDATION_FAILED', () => { code: 'VALIDATION_FAILED', details: { fields: [{ field: 'name', code: 'required' }] }, }); - const normalised = normaliseClientError(err) as ValidationError; + const normalised = normaliseClientError(err) as DataApiValidationError; expect(normalised.validationErrors).toEqual([{ field: 'name', message: 'required' }]); }); @@ -72,7 +72,7 @@ describe('normaliseClientError — VALIDATION_FAILED', () => { code: 'VALIDATION_FAILED', details: { fields: [{ message: 'something is wrong' }] }, }); - const normalised = normaliseClientError(err) as ValidationError; + const normalised = normaliseClientError(err) as DataApiValidationError; expect(normalised.validationErrors).toEqual([]); }); @@ -83,7 +83,7 @@ describe('normaliseClientError — VALIDATION_FAILED', () => { name: 'ValidationError', fields: [{ field: 'amount', message: 'Amount must be positive' }], }); - const normalised = normaliseClientError(err) as ValidationError; + const normalised = normaliseClientError(err) as DataApiValidationError; expect(normalised.validationErrors).toEqual([{ field: 'amount', message: 'Amount must be positive' }]); }); diff --git a/packages/plugin-chatbot/package.json b/packages/plugin-chatbot/package.json index 80affaabcb..943e9a4387 100644 --- a/packages/plugin-chatbot/package.json +++ b/packages/plugin-chatbot/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.typetests.json", "lint": "eslint ." }, "dependencies": { @@ -38,6 +38,7 @@ "@object-ui/react": "workspace:*", "@object-ui/types": "workspace:*", "@radix-ui/react-slot": "^1.3.3", + "@objectstack/spec": "^17.0.0-rc.1", "@radix-ui/react-use-controllable-state": "^1.2.6", "ai": "^7.0.37", "class-variance-authority": "^0.7.1", diff --git a/packages/plugin-chatbot/src/__tests__/spec-symbol-batch6.test.ts b/packages/plugin-chatbot/src/__tests__/spec-symbol-batch6.test.ts new file mode 100644 index 0000000000..04f3ef917c --- /dev/null +++ b/packages/plugin-chatbot/src/__tests__/spec-symbol-batch6.test.ts @@ -0,0 +1,243 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `@object-ui/plugin-chatbot` ↔ `@objectstack/spec` symbol-collision guards + * (objectui#3160, objectstack#4115 ledger batch 6). + * + * Four symbols here wore names the spec owns, and they split two ways: + * + * - `PendingActionStatus` / `PendingActionRow` were hand transcriptions of the + * `IAIService` contract the REST route serialises. Both are re-exports now, + * and the assertions below pin the three drifts the copies carried, each of + * which had DISABLED a compile-time check rather than merely differed from + * one. + * + * - `Tool` / `MessageContent` live in `src/elements/`, which is not objectui's + * authored surface: it is Vercel AI Elements (https://elements.ai-sdk.dev, + * MIT) — plus two Shadcn primitives under `elements/ui/` that + * `@object-ui/components` does not ship yet — vendored by the same + * copy-into-source model as the Shadcn no-touch zone in + * `@object-ui/components` and re-synced from upstream. + * Those names ARE the upstream component API — renaming them is undone by + * the next re-sync and breaks `` for anyone reading + * upstream's docs — so the guard skips the directory + * (`SKIP_PATH_SEGMENTS` in scripts/check-spec-symbol-derivation.mjs) exactly + * as it already skips `components/src/ui/`. + * + * A path skip is broader than an ALLOW entry, and the hole it opens is an + * objectui-AUTHORED file dropped into that directory and silently unscanned. + * `the vendored directory stays vendored` below is what closes it. + */ + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; +import { createRequire } from 'node:module'; +import { readFileSync, readdirSync } from 'node:fs'; +import { resolve, dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import type { PendingActionRow, PendingActionStatus } from '../usePendingActions'; +import type { + PendingActionRow as SpecPendingActionRow, + PendingActionStatus as SpecPendingActionStatus, +} from '@objectstack/spec/contracts'; + +/** Every name `@objectstack/spec` exports from any subpath — types AND values. */ +function specExportNames(): Set { + const require = createRequire(import.meta.url); + const pkgPath = require.resolve('@objectstack/spec/package.json'); + const pkgDir = dirname(pkgPath); + const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')) as { + exports?: Record; + }; + + const files: string[] = []; + for (const cond of Object.values(pkg.exports ?? {})) { + if (typeof cond !== 'object' || cond === null) continue; + const dts = cond.import?.types ?? cond.require?.types; + if (dts) files.push(resolve(pkgDir, dts)); + } + + const program = ts.createProgram(files, { + noEmit: true, + skipLibCheck: true, + strict: false, + target: ts.ScriptTarget.ESNext, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + }); + const checker = program.getTypeChecker(); + + const names = new Set(); + for (const file of files) { + const sf = program.getSourceFile(file); + if (!sf) continue; + const moduleSymbol = checker.getSymbolAtLocation(sf); + if (!moduleSymbol) continue; + for (const exported of checker.getExportsOfModule(moduleSymbol)) names.add(exported.getName()); + } + return names; +} + +const SPEC_NAMES = specExportNames(); + +describe('the spec export-name probe itself works', () => { + it('reads a non-trivial number of names', () => { + expect(SPEC_NAMES.size).toBeGreaterThan(1000); + }); + + it('sees TYPE-only exports, not just runtime values', () => { + expect(SPEC_NAMES.has('PendingActionRow')).toBe(true); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* The vendored directory */ +/* -------------------------------------------------------------------------- */ + +const ELEMENTS_DIR = resolve(dirname(fileURLToPath(import.meta.url)), '..', 'elements'); +/** + * The vendored-provenance banner every file there carries. Two upstreams are + * represented — `vercel/ai-elements` for the chat primitives and `shadcn/ui` + * for the two `ui/` primitives `@object-ui/components` does not ship yet — and + * both banners say the same load-bearing thing: sourced from elsewhere, do not + * edit, re-synced from upstream. That, not the specific project, is what makes + * the directory not-ours and therefore skippable by the spec-symbol guard. + */ +const VENDOR_BANNER = /Sourced from \S+ \(http[^)]+\) — MIT\./; +/** The one objectui-authored file there: a barrel that re-exports and nothing else. */ +const BARREL = 'index.ts'; + +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) walk(full, out); + else if (/\.tsx?$/.test(entry.name)) out.push(full); + } + return out; +} + +describe('the vendored directory stays vendored', () => { + const files = walk(ELEMENTS_DIR); + + it('finds the files the guard is skipping', () => { + // If this drops to zero the skip has stopped covering anything and the + // SKIP_PATH_SEGMENTS entry is stale — delete it rather than leave a path + // reserved for a future fork. + expect(files.length).toBeGreaterThan(5); + expect(files.some((f) => f.endsWith('tool.tsx'))).toBe(true); + expect(files.some((f) => f.endsWith('message.tsx'))).toBe(true); + }); + + it.each(files.map((f) => [f.slice(ELEMENTS_DIR.length + 1), f]))( + '%s is upstream, not ours', + (rel, full) => { + const text = readFileSync(full, 'utf8'); + if (rel === BARREL) { + // The barrel is objectui's, so it must stay a pure re-export: a + // DECLARATION added here would be authored code sitting inside the + // skipped path, invisible to the spec-symbol guard. + const body = text + .split('\n') + .filter((l) => l.trim() && !l.trim().startsWith('*') && !l.trim().startsWith('/*')) + .filter((l) => !l.trim().startsWith('//')); + for (const line of body) { + expect( + /^export \* from '\.\/[\w-]+';$/.test(line.trim()), + `${rel} declares something of its own (\`${line.trim()}\`). That file is inside ` + + `SKIP_PATH_SEGMENTS, so the spec-symbol guard cannot see it. Move the ` + + `declaration to an objectui-authored module outside src/elements/.`, + ).toBe(true); + } + return; + } + expect( + VENDOR_BANNER.test(text), + `${rel} is inside src/elements/, which scripts/check-spec-symbol-derivation.mjs ` + + `skips BECAUSE everything there is vendored from upstream. This file carries no ` + + `vendor banner, so it is either objectui-authored code hiding from the guard, or ` + + `a re-sync that dropped the banner. Move it out, or restore the banner.`, + ).toBe(true); + }, + ); + + it('the two collisions the skip covers are still spec names', () => { + // A reverse pin, per the pattern batch 5 used for `PerformanceConfig`: the + // guard's SKIP comment names `Tool` and `MessageContent` as the collisions + // it is waving through. If the spec retires either name that note is stale + // and the entry deserves a fresh read — the workaround must not outlive its + // stated reason. + for (const owned of ['Tool', 'MessageContent']) { + expect( + SPEC_NAMES.has(owned), + `@objectstack/spec no longer exports \`${owned}\` — the SKIP_PATH_SEGMENTS note ` + + `in scripts/check-spec-symbol-derivation.mjs still cites it as one of the two ` + + `collisions in src/elements/. Re-read the entry.`, + ).toBe(true); + } + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Compile-time pins. A violation is a `tsc` error, not a runtime failure. */ +/* Compiled by this package's `tsconfig.typetests.json` (objectui#3181). */ +/* -------------------------------------------------------------------------- */ + +type Assert = T; +type IsAny = 0 extends 1 & T ? true : false; +/** The `unknown` erasure the `any` probe reports `false` for (objectui#3155). */ +type IsUnknown = [unknown] extends [T] ? ([T] extends [unknown] ? true : false) : false; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; +/** objectstack#4075: an index signature absorbs every excess key. */ +type HasIndexSignature = string extends keyof T ? true : false; + +describe('the pending-action row and status ARE the spec contract', () => { + it('is pinned at compile time', () => { + type _RowNotAny = Assert, false>>; + type _RowNotUnknown = Assert, false>>; + type _StatusNotAny = Assert, false>>; + + type _RowIsSpec = Assert>; + type _StatusIsSpec = Assert>; + + // Drift 1 — the copy declared `status: PendingActionStatus | string`. A + // union with `string` ABSORBS the literals, so that annotation carried no + // information at all: `statusesForTab` could have returned a status the + // server has never heard of and nothing would have said so. + type _StatusIsNotString = Assert>; + + // Drift 2 — `[k: string]: unknown`. This is the objectstack#4075 mechanism: + // with it, ANY structural comparison against the spec answers "identical", + // however far the copy has drifted, which is why the guard and not a parity + // test is what found this one. + type _NoIndexSignature = Assert, false>>; + + // Drift 3 — `created_at` / `updated_at`, which the contract does not carry + // and nothing in this repo reads. If the inbox ever needs them the fix is a + // spec change, not a local widening. + type _NoCreatedAt = Assert>; + type _NoUpdatedAt = Assert>; + + // The vocabulary the inbox's tabs and badges are built on. + type _Vocabulary = Assert< + Equal + >; + + expect(true).toBe(true); + }); + + it('still names every status the inbox renders a badge for', () => { + // A cheap runtime net over the compile-time pin: if the spec RETIRES a + // status, this fails here rather than as an unstyled badge in the queue. + const all: PendingActionStatus[] = ['pending', 'approved', 'executed', 'failed', 'rejected']; + expect(new Set(all).size).toBe(5); + }); +}); diff --git a/packages/plugin-chatbot/src/usePendingActions.ts b/packages/plugin-chatbot/src/usePendingActions.ts index 7bbe70ca10..a5edfedeaf 100644 --- a/packages/plugin-chatbot/src/usePendingActions.ts +++ b/packages/plugin-chatbot/src/usePendingActions.ts @@ -24,40 +24,38 @@ import * as React from 'react'; -export type PendingActionStatus = - | 'pending' - | 'approved' - | 'executed' - | 'failed' - | 'rejected'; +import type { + PendingActionRow, + PendingActionStatus, +} from '@objectstack/spec/contracts'; /** - * Wire-format row returned by - * `GET /api/v1/ai/pending-actions` and friends. Mirrors the - * `ai_pending_action` object schema declared in - * `@objectstack/service-ai`. + * Lifecycle of a pending action proposal, and the row `GET + * /api/v1/ai/pending-actions` returns — THE spec types, re-exported + * (objectui#3160, objectstack#4115 ledger batch 6). + * + * `@objectstack/spec/contracts` declares both as the contract of + * `IAIService.proposePendingAction` / `.listPendingActions`, which is exactly + * what the REST route serialises; the copies that used to live here were a + * hand transcription of the same rows and had drifted in three ways, each of + * which silently disabled a compile-time check: + * + * - `status: PendingActionStatus | string` — a union with `string` ABSORBS the + * literals, so the type conveyed nothing at all and `statusesForTab` could + * have returned a status the server has never heard of; + * - `[k: string]: unknown` — the objectstack#4075 mechanism: an index + * signature makes any structural comparison against the spec answer + * "identical" no matter how far the copy drifts; + * - `created_at` / `updated_at`, which the contract does not carry and no + * consumer in this repo reads. If the inbox ever needs them, the fix is to + * model them in the spec, not to re-widen the row here. + * + * `| null` was dropped with the copy for the same reason: it described what a + * nullable SQL column might serialise to, not what the contract promises, and + * every reader here (`formatRelative`, `safeParseJson`) already accepts + * `null | undefined` on its own parameter. */ -export interface PendingActionRow { - id: string; - conversation_id?: string | null; - message_id?: string | null; - object_name: string; - action_name: string; - tool_name: string; - /** JSON-encoded string. Consumers typically `JSON.parse` to render. */ - tool_input: string; - status: PendingActionStatus | string; - result?: string | null; - error?: string | null; - rejection_reason?: string | null; - proposed_by?: string | null; - decided_by?: string | null; - proposed_at?: string; - decided_at?: string | null; - created_at?: string; - updated_at?: string; - [k: string]: unknown; -} +export type { PendingActionRow, PendingActionStatus }; /** * Successful approval outcome returned by diff --git a/packages/plugin-chatbot/tsconfig.typetests.json b/packages/plugin-chatbot/tsconfig.typetests.json new file mode 100644 index 0000000000..42ba287013 --- /dev/null +++ b/packages/plugin-chatbot/tsconfig.typetests.json @@ -0,0 +1,34 @@ +{ + // Compiles the test file whose load-bearing content is compile-time type + // assertions, so those assertions are actually checked by CI (objectui#3181). + // + // `src/__tests__/spec-symbol-batch6.test.ts` states its claims as TYPES — + // `Assert>` is a compile error or it is nothing. The + // package's own `tsconfig.json` is the BUILD config and excludes + // `**/*.test.ts` (correctly — a test file must not emit into dist), and CI's + // only type gate drives that config, so without this project the pins would + // be the "declared != enforced" landmine objectstack#4115 exists to remove, + // sitting inside the guard for it. + // + // Chained from this package's `type-check` script, which is what the CI + // `Type Check` job runs; scripts/check-type-check-coverage.mjs enforces the + // chaining. Explicit include list, not a glob: this package is still in that + // script's TEST_DEBT, so a glob would drag in the rest of the test tree. + "extends": "../../tsconfig.json", + "compilerOptions": { + // A checking project, never an emitting one. + "noEmit": true, + "composite": false, + "declaration": false, + "lib": ["ES2020", "DOM"], + // The parity test resolves `@objectstack/spec`'s own `.d.ts` files off disk + // (createRequire / readFileSync / node:path) and walks `src/elements/` to + // prove that directory is still fully vendored. + "types": ["node"], + // Drop the root tsconfig's source-tree `paths` so `@objectstack/spec` and + // the `@object-ui/*` workspace deps resolve through the real dependency + // graph rather than through sibling `src/`. + "paths": {} + }, + "include": ["src/__tests__/spec-symbol-batch6.test.ts"] +} diff --git a/packages/plugin-list/package.json b/packages/plugin-list/package.json index 08c5e518d9..ab7cf5e83d 100644 --- a/packages/plugin-list/package.json +++ b/packages/plugin-list/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.typetests.json", "lint": "eslint ." }, "dependencies": { @@ -42,6 +42,7 @@ "@object-ui/permissions": "workspace:^", "@object-ui/react": "workspace:^", "@object-ui/types": "workspace:^", + "@objectstack/spec": "^17.0.0-rc.1", "react": "^18.0.0 || ^19.0.0", "react-dom": "^18.0.0 || ^19.0.0" }, diff --git a/packages/plugin-list/src/__tests__/spec-symbol-batch6.test.tsx b/packages/plugin-list/src/__tests__/spec-symbol-batch6.test.tsx new file mode 100644 index 0000000000..8fff31ab9e --- /dev/null +++ b/packages/plugin-list/src/__tests__/spec-symbol-batch6.test.tsx @@ -0,0 +1,238 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `@object-ui/plugin-list` ↔ `@objectstack/spec` symbol-collision guards + * (objectui#3160, objectstack#4115 ledger batch 6). + * + * Three symbols here wore names the spec owns, and the verdicts differ by + * KIND, not by cluster: + * + * - `ViewTab` was a hand copy of `ViewTabSchema` under the spec's own name, + * drifted in three places. It is derived now — from the schema's INPUT side, + * because `pinned` / `isDefault` / `visible` carry `.default()`s and this + * component is handed authored metadata, not parsed output. + * + * - `ListView` and `UserFilters` are the RENDERERS of two spec types, and both + * are ALLOW entries in scripts/check-spec-symbol-derivation.mjs. The test + * applied is the one the guard's `AuthProvider` entry states — not "React + * components are exempt" (batch 5 renamed `Field` → `FieldContainer`), but + * "would the next session read THIS declaration as canonical for the spec's + * shape?". Neither declares a shape at all: each takes the spec's type as a + * prop, so the two layers are joined at the declaration instead of restated. + * The assertions below are what keeps that claim true — they fail if either + * component stops consuming the spec-derived type, at which point the + * exemption's reason has expired and the symbol needs re-triage. + */ + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; +import { createRequire } from 'node:module'; +import { readFileSync } from 'node:fs'; +import { resolve, dirname } from 'node:path'; + +import { ListView } from '../ListView'; +import type { ListViewProps } from '../ListView'; +import { UserFilters } from '../UserFilters'; +import type { UserFiltersProps } from '../UserFilters'; +import type { ViewTab } from '../components/TabBar'; + +import type { ListViewSchema } from '@object-ui/types'; +import type { + ListView as SpecListView, + UserFilters as SpecUserFilters, + ViewTab as SpecViewTab, + ViewTabSchema as SpecViewTabSchema, +} from '@objectstack/spec/ui'; + +/** Every name `@objectstack/spec` exports from any subpath — types AND values. */ +function specExportNames(): Set { + const require = createRequire(import.meta.url); + const pkgPath = require.resolve('@objectstack/spec/package.json'); + const pkgDir = dirname(pkgPath); + const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')) as { + exports?: Record; + }; + + const files: string[] = []; + for (const cond of Object.values(pkg.exports ?? {})) { + if (typeof cond !== 'object' || cond === null) continue; + const dts = cond.import?.types ?? cond.require?.types; + if (dts) files.push(resolve(pkgDir, dts)); + } + + const program = ts.createProgram(files, { + noEmit: true, + skipLibCheck: true, + strict: false, + target: ts.ScriptTarget.ESNext, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + }); + const checker = program.getTypeChecker(); + + const names = new Set(); + for (const file of files) { + const sf = program.getSourceFile(file); + if (!sf) continue; + const moduleSymbol = checker.getSymbolAtLocation(sf); + if (!moduleSymbol) continue; + for (const exported of checker.getExportsOfModule(moduleSymbol)) names.add(exported.getName()); + } + return names; +} + +const SPEC_NAMES = specExportNames(); + +describe('the spec export-name probe itself works', () => { + it('reads a non-trivial number of names', () => { + expect(SPEC_NAMES.size).toBeGreaterThan(1000); + }); + + it('sees TYPE-only exports, not just runtime values', () => { + expect(SPEC_NAMES.has('ViewTab')).toBe(true); + }); +}); + +describe('the two ALLOW entries still describe live collisions', () => { + // An ALLOW entry that excuses nothing is stale — the guard's own third + // ratchet says so. These names being spec-owned is the whole premise. + it.each([ + ['ListView', 'the authored list-view metadata document'], + ['UserFilters', 'the ADR-0047 quick-filter configuration'], + ])('the spec still owns `%s` (%s)', (name) => { + expect( + SPEC_NAMES.has(name), + `@objectstack/spec no longer exports \`${name}\`. The ALLOW entry in ` + + `scripts/check-spec-symbol-derivation.mjs excuses a collision that no longer ` + + `exists — delete the entry so the name cannot be re-forked under an inherited ` + + `exemption.`, + ).toBe(true); + }); +}); + +describe('the renderers are components, and the spec names are metadata', () => { + it('`ListView` is a React component, not a shape declaration', () => { + // `React.forwardRef` returns an exotic object, not a function. + expect(typeof ListView).toBe('object'); + expect(ListView).toHaveProperty('render'); + }); + + it('`UserFilters` is a React component, not a shape declaration', () => { + expect(typeof UserFilters).toBe('function'); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Compile-time pins. A violation is a `tsc` error, not a runtime failure. */ +/* Compiled by this package's `tsconfig.typetests.json` (objectui#3181). */ +/* -------------------------------------------------------------------------- */ + +type Assert = T; +type Extends = [A] extends [B] ? true : false; +type IsAny = 0 extends 1 & T ? true : false; +/** The `unknown` erasure the `any` probe reports `false` for (objectui#3155). */ +type IsUnknown = [unknown] extends [T] ? ([T] extends [unknown] ? true : false) : false; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; +type IsOptional = undefined extends T[K] ? true : false; + +/** The spec's own authoring type for a view tab — what `.parse()` ACCEPTS. */ +type SpecViewTabInput = (typeof SpecViewTabSchema)['_zod']['input']; + +describe('ViewTab derives from the spec schema, on the authoring side', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + type _SpecNotUnknown = Assert, false>>; + + type _IsTheSpecInput = Assert>; + + // Input, not output, and here is the evidence: the PARSED type requires the + // three defaulted keys, so re-exporting `SpecViewTab` would have made a + // stored `{ name: 'open', label: 'Open' }` unrepresentable — the + // `_input`/`_output` trap the guard's header warns about, and the one that + // already bit `ObjectFieldGroup` (objectui#3169) and `OfflineConfig` + // (objectui#3199). + type _ParsedRequiresPinned = Assert, false>>; + type _ParsedRequiresVisible = Assert, false>>; + type _AuthoredMayOmitPinned = Assert>; + type _AuthoredMayOmitVisible = Assert>; + + // Same key set as the parsed type — only optionality differs. A key the + // spec adds appears here; a key it retires disappears. + type _NoLocalKeys = Assert, never>>; + type _NoMissingKeys = Assert, never>>; + + // The three drifts the hand copy carried, pinned as fixed: + // 1. `label` was REQUIRED locally; the spec makes it optional (`name` is + // the identifier, and TabBar falls back to it). + type _LabelIsOptional = Assert>; + // 2. `filter` was `any`, so a mistyped operator was unreportable. + type _FilterIsNotAny = Assert, false>>; + // 3. `visible` accepted `string | boolean` — a renderer-side tolerance for + // a shape no producer emits (AGENTS.md #12). + type _VisibleIsBoolean = Assert, boolean>>; + + expect(true).toBe(true); + }); +}); + +describe('ListView RENDERS the spec metadata rather than restating it', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + type _SpecNotUnknown = Assert, false>>; + + // The spec's `ListView` is a metadata DOCUMENT — it has the authoring keys + // and nothing renderer-shaped. Pinned so "it is a different layer" stays a + // fact rather than an assertion in a comment. + type _SpecIsMetadata = Assert>; + + // The component's own props bind that layer: `schema` is `ListViewSchema` + // from `@object-ui/types` — the declared dialect that derives from the + // spec's zod node (its own ALLOW entry, pinned by list-view-spec-parity). + // THIS is the reason the collision is excused: the renderer consumes the + // authored type, it does not declare a rival one. If this ever stops + // holding, the ALLOW entry has lost its reason. + type _PropsBindTheSchema = Assert>; + + // And the local export declares no view shape of its own: everything on + // `ListViewProps` beyond `schema` is renderer plumbing (callbacks, initial + // UI state), never metadata. + type _SchemaIsTheOnlyMetadata = Assert>; + + expect(true).toBe(true); + }); +}); + +describe('UserFilters RENDERS the spec quick-filter config', () => { + it('is pinned at compile time', () => { + type _SpecNotAny = Assert, false>>; + type _SpecNotUnknown = Assert, false>>; + + // The spec's `UserFilters` is the authored config: an element style plus + // the fields and tab presets exposed to end users. + type _SpecIsConfig = Assert>; + type _SpecHasTabs = Assert>; + + // The component takes that config as a PROP — via + // `NonNullable`, the objectui view type that + // derives from the same spec node. Nothing here re-declares the config, so + // there is nothing here to drift from it; that is the ALLOW entry's reason, + // and this line is what keeps it true. + type _ConfigCarriesTheElement = Assert< + Equal<'element' extends keyof UserFiltersProps['config'] ? true : false, true> + >; + type _ConfigCarriesTheFields = Assert< + Equal<'fields' extends keyof UserFiltersProps['config'] ? true : false, true> + >; + type _ConfigIsNotAny = Assert, false>>; + + expect(true).toBe(true); + }); +}); diff --git a/packages/plugin-list/src/components/TabBar.tsx b/packages/plugin-list/src/components/TabBar.tsx index 91e9ef46a1..f3046122e3 100644 --- a/packages/plugin-list/src/components/TabBar.tsx +++ b/packages/plugin-list/src/components/TabBar.tsx @@ -16,18 +16,31 @@ import { DropdownMenuItem, } from '@object-ui/components'; import { icons, ChevronDown, type LucideIcon } from 'lucide-react'; +import type { ViewTabSchema } from '@objectstack/spec/ui'; -export interface ViewTab { - name: string; - label: string; - icon?: string; - view?: string; - filter?: any; - order?: number; - pinned?: boolean; - isDefault?: boolean; - visible?: string | boolean; -} +/** + * One tab in a multi-tab list view — derived from the spec's `ViewTabSchema` + * (objectui#3160, objectstack#4115 ledger batch 6). + * + * Until then this was a hand copy wearing the spec's own name, drifted in three + * places: `label` was REQUIRED (the spec makes it optional — `name` is the + * identifier), `filter` was `any` (the spec models `ViewFilterRule[]`, so a + * mistyped operator was unreportable), and `visible` accepted `string | boolean` + * — a renderer-side tolerance for a shape the spec never emits, which is the + * lenient fallback AGENTS.md #12 bans. `@object-ui/types` has re-exported the + * spec's `ViewTab` under that name all along, so the fork and the real thing + * were already importable side by side. + * + * Bound to the AUTHORING (`input`) side, not `z.infer`: `pinned` / `isDefault` / + * `visible` all carry `.default()`s, so the PARSED type makes three keys + * required and a host handing this component stored view metadata + * (`{ name: 'open' }`) could not express it. The rule recorded on + * objectstack#4115 — writing this metadata → input, reading what has already + * been parsed → `z.infer`. Reached through the schema's own `_zod` carrier + * rather than `z.input` so this package takes no zod dependency; same technique + * and fuller rationale in `packages/react/src/spec-input.ts`. + */ +export type ViewTab = (typeof ViewTabSchema)['_zod']['input']; export interface TabBarProps { tabs: ViewTab[]; @@ -50,12 +63,18 @@ function resolveIcon(iconName?: string): LucideIcon | null { } /** - * Filter visible tabs: exclude tabs where visible is 'false' or boolean false. + * Filter visible tabs: exclude tabs where `visible` is false. * Pinned tabs are always included regardless of other filtering. + * + * The `!== 'false'` half of this predicate went with the hand-copied `ViewTab` + * above (objectui#3160): the spec types `visible` as a boolean and nothing in + * the platform emits the STRING `'false'`, so the extra comparison was a + * renderer-side accommodation for a shape no producer writes — and with the + * derived type it is a compile error rather than dead code. */ function getVisibleTabs(tabs: ViewTab[]): ViewTab[] { return tabs - .filter(tab => tab.pinned || (tab.visible !== 'false' && tab.visible !== false)) + .filter(tab => tab.pinned || tab.visible !== false) .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)); } diff --git a/packages/plugin-list/tsconfig.typetests.json b/packages/plugin-list/tsconfig.typetests.json new file mode 100644 index 0000000000..69efe9e344 --- /dev/null +++ b/packages/plugin-list/tsconfig.typetests.json @@ -0,0 +1,35 @@ +{ + // Compiles the test file whose load-bearing content is compile-time type + // assertions, so those assertions are actually checked by CI (objectui#3181). + // + // `src/__tests__/spec-symbol-batch6.test.tsx` states its claims as TYPES — + // `Assert>` is a compile error or it is nothing. The + // package's own `tsconfig.json` is the BUILD config and excludes + // `**/*.test.tsx` (correctly — a test file must not emit into dist), and CI's + // only type gate drives that config, so without this project the pins would + // be the "declared != enforced" landmine objectstack#4115 exists to remove, + // sitting inside the guard for it. + // + // Chained from this package's `type-check` script, which is what the CI + // `Type Check` job runs; scripts/check-type-check-coverage.mjs enforces the + // chaining. Explicit include list, not a glob: this package is still in that + // script's TEST_DEBT, so a glob would drag in the rest of the test tree. + "extends": "../../tsconfig.json", + "compilerOptions": { + // A checking project, never an emitting one. + "noEmit": true, + "composite": false, + "declaration": false, + "jsx": "react-jsx", + "lib": ["ES2020", "DOM"], + // The parity test resolves `@objectstack/spec`'s own `.d.ts` files off disk + // (createRequire / readFileSync / node:path) to read the spec's export + // names through the TypeScript checker. + "types": ["node"], + // Drop the root tsconfig's source-tree `paths` so `@objectstack/spec` and + // the `@object-ui/*` workspace deps resolve through the real dependency + // graph rather than through sibling `src/`. + "paths": {} + }, + "include": ["src/__tests__/spec-symbol-batch6.test.tsx"] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f8e52d940..eefcf7e567 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1153,10 +1153,10 @@ importers: '@objectstack/client': specifier: ^17.0.0-rc.1 version: 17.0.0-rc.1(ai@7.0.37(zod@4.4.3)) - devDependencies: '@objectstack/spec': specifier: ^17.0.0-rc.1 version: 17.0.0-rc.1(ai@7.0.37(zod@4.4.3)) + devDependencies: tsup: specifier: ^8.5.1 version: 8.5.1(@microsoft/api-extractor@7.58.2(@types/node@26.1.1))(@swc/core@1.15.33)(jiti@2.7.0)(postcss@8.5.24)(tsx@4.23.1)(typescript@6.0.3)(yaml@2.9.0) @@ -1531,6 +1531,9 @@ importers: '@object-ui/types': specifier: workspace:* version: link:../types + '@objectstack/spec': + specifier: ^17.0.0-rc.1 + version: 17.0.0-rc.1(ai@7.0.37(zod@4.4.3)) '@radix-ui/react-slot': specifier: ^1.3.3 version: 1.3.3(@types/react@19.2.17)(react@19.2.8) diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index 52498f40af..6331be86aa 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -173,6 +173,38 @@ const ALLOW = { "where a stored row can be null. A row is not a response.", issue: 4115, }, + // Two RENDERERS in @object-ui/plugin-list, judged by the AuthProvider rule + // above and NOT by "components are exempt" — the sibling `ViewTab` in the same + // package was a hand copy under a spec name and was derived (objectui#3160). + "@object-ui/plugin-list:ListView": { + reason: + "The RENDERER of the spec type, not a second declaration of it. The spec's `ListView` " + + "is authored view METADATA (`z.infer`, type-only); this is the " + + "React component that draws it, and its own props bind that metadata at the declaration " + + "— `ListViewProps.schema` is `ListViewSchema` from `@object-ui/types`, itself the " + + "declared dialect two entries up. So the two layers are joined here, not confused: there " + + "is no shape to drift, and `` cannot be read as canonical for a " + + "metadata SHAPE the way `AuthProviderConfig` could. The repo already disambiguates from " + + "the other side — `@object-ui/types` re-exports the spec's type as `SpecListView` — and " + + "renaming the package's headline export would rewrite every consumer's JSX for zero " + + "defect, the AuthProvider judgement exactly. Pinned by " + + "packages/plugin-list/src/__tests__/spec-symbol-batch6.test.tsx, which fails if the " + + "spec's `ListView` stops being authored metadata or if this export stops being a " + + "component that consumes it.", + issue: 4115, + }, + "@object-ui/plugin-list:UserFilters": { + reason: + "Same judgement as `ListView` directly above, and the same package. The spec's " + + "`UserFilters` is the ADR-0047 quick-filter CONFIG (`{ element, fields, tabs, … }`, " + + "type-only); this is the filter bar that renders it, and it takes that config as a prop " + + "— `UserFiltersProps.config` is `NonNullable`, so the " + + "spec's shape is what this component is typed against rather than something it restates. " + + "Nothing here declares a filter shape, so nothing here can drift from one. Pinned by the " + + "same test file, which asserts `UserFiltersProps['config']` still accepts the spec's " + + "authored `UserFilters`.", + issue: 4115, + }, }; // ── Untriaged collisions (the ledger) ──────────────────────────────────────── @@ -256,24 +288,6 @@ const ALLOW = { // Compare `_input` too before touching a schema const. const DEBT_ISSUE = 4115; const DEBT = { - "@object-ui/data-objectstack": [ - "CacheStats", - "DroppedFieldsEvent", - "MetadataSaveOptions", - "SecurityPolicy", - "ValidationError", - ], - "@object-ui/plugin-chatbot": [ - "MessageContent", - "PendingActionRow", - "PendingActionStatus", - "Tool", - ], - "@object-ui/plugin-list": [ - "ListView", - "UserFilters", - "ViewTab", - ], "@object-ui/types": [ "JoinedReportBlock", "NavigationItem", @@ -318,7 +332,24 @@ const DEBT = { // Files under these paths are not objectui's own authored surface. // - `ui/` is the Shadcn no-touch zone (AGENTS.md #7): upstream 3rd-party files // overwritten by sync scripts, so a collision there is not ours to fix. -const SKIP_PATH_SEGMENTS = ["components/src/ui/"]; +// - `plugin-chatbot/src/elements/` is the same class one package over: Vercel +// AI Elements (https://elements.ai-sdk.dev, MIT) plus two Shadcn primitives +// `@object-ui/components` does not ship yet, vendored by the identical +// copy-into-source model and re-synced from upstream. Every file there +// carries the banner saying so. Two of its exports collide — +// `Tool` (a `` shell for a tool call; the spec's is an agent +// TOOL DEFINITION) and `MessageContent` (a styled `
`; the spec's is an +// AI message payload) — and neither is renameable: the names ARE the +// upstream component API, so a rename is reverted by the next re-sync and +// breaks `` for anyone following upstream docs. +// Skipping the directory rather than ALLOW-ing the two names is deliberate: +// a future re-sync must not fail an unrelated PR over a third vendored name +// nobody here is allowed to rename either. The hole that opens — an +// objectui-AUTHORED file hiding under the skip — is closed by +// packages/plugin-chatbot/src/__tests__/spec-symbol-batch6.test.ts, which +// fails if any file there stops carrying the vendored banner. +// (objectui#3160, objectstack#4115 ledger batch 6.) +const SKIP_PATH_SEGMENTS = ["components/src/ui/", "plugin-chatbot/src/elements/"]; const isSpecModule = (m) => m === "@objectstack/spec" || m.startsWith("@objectstack/spec/");