diff --git a/.changeset/6559-expression-user-input-contract.md b/.changeset/6559-expression-user-input-contract.md new file mode 100644 index 0000000000..0606632735 --- /dev/null +++ b/.changeset/6559-expression-user-input-contract.md @@ -0,0 +1,46 @@ +--- +'@object-ui/app-shell': minor +--- + +**Breaking (compile-time only):** `buildExpressionUser`'s PARAMETER is now the session +contract — `ExpressionUserSession | null | undefined` instead of `unknown` — so every call +site is checked against it (objectui#6559). + +objectui#6551 narrowed the CAST the normaliser read its input through, so the module began +DECLARING what a signed-in session is: `id`, `name` and `email` required, mirroring +`@object-ui/auth`'s `AuthUser`. The parameter behind that cast stayed `unknown`, so the +declaration bound nothing — every call site satisfied it vacuously and +`buildExpressionUser({ name: 'B', email: 'b@c.d' })` still compiled. A declaration nothing +checks is indistinguishable from no declaration at all (AGENTS.md #0.1). The cast is gone +and the shape is stated once, on the parameter, so the declaration and the check are the +same statement rather than two that merely agree. + +WHAT BREAKS, AND FOR WHOM. This tightens a signature published from the package entry +(`packages/app-shell/src/index.ts`), so an external caller that passes an unchecked or +under-declared value stops compiling on upgrade. That is accepted (maintainer ruling +2026-08-27, option A): the only calls it refuses are calls that were never conformant with +the contract the module already declared. It ships as `minor`, not `major` — objectui's +major tracks `@objectstack`'s, so its own breaking changes ship as a minor with the break +written down (`scripts/check-changeset-no-major.mjs`). ⛔ Keeping `unknown` and ⛔ adding a +second, wider entry point were both declined. + +NO RUNTIME BEHAVIOUR MOVES. All four in-repo production call sites pass `useAuth().user`, +typed `AuthUser | null`, and type cleanly unchanged — two in `console/AppContent.tsx`, one +in `views/RecordFormPage.tsx`, one in `apps/console`'s `InternalFormRoute.tsx`. The body is +byte-equivalent: the same keys, the same `??` defaults, the same anonymous branch. ⛔ No +consumer-side fallback was added; `id: u.id ?? null` remains the rejected shape (triage +ruling 2026-08-26), because a lenient default in the consumer is what AGENTS.md #0.1 +forbids and it silently equates "signed in, no id" with "signed out". + +Note for callers holding the SPEC's `AuthUser` rather than `@object-ui/auth`'s: the spec +type is an `interface` with no index signature, and TypeScript infers an implicit index +signature for type aliases only, so it is not assignable to a contract declaring +`[key: string]: unknown`. `@object-ui/auth`'s `AuthUser` extends it and adds that index +signature, which is what every call site here passes. + +Pinned by the new `expressionUser.parameterContract.types.test.ts`, compiled by the +package's `tsconfig.test.json` (chained off `type-check`, which CI runs). Its refusals are +`@ts-expect-error` directives, so a re-widened parameter makes them UNUSED and TS2578 turns +the type-check red; a type equation on `Parameters[0]` reds +alongside them. Every refusal is routed through a non-fresh value, so what is measured is +the parameter and not excess-property freshness. diff --git a/packages/app-shell/src/providers/expressionUser.parameterContract.types.test.ts b/packages/app-shell/src/providers/expressionUser.parameterContract.types.test.ts new file mode 100644 index 0000000000..c0b48394a6 --- /dev/null +++ b/packages/app-shell/src/providers/expressionUser.parameterContract.types.test.ts @@ -0,0 +1,193 @@ +/** + * 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. + */ + +/** + * Type-level pin for objectui#6559 — `buildExpressionUser`'s PARAMETER is the + * session contract, so every call site is checked against it. + * + * ## What the defect was + * + * objectui#6551 narrowed the CAST the normaliser read its input through, so the + * module began DECLARING what a signed-in session is (`id` / `name` / `email` + * required, mirroring the spec's `AuthUser`). The parameter behind that cast + * stayed `unknown`, so the declaration bound nothing: every call site satisfied + * it vacuously, and `buildExpressionUser({ name: 'B', email: 'b@c.d' })` still + * compiled. A declaration nothing checks is indistinguishable from no + * declaration — AGENTS.md #0.1. That gap is what this card closes, by moving + * the declaration from the cast onto the parameter. + * + * Maintainer ruling 2026-08-27 (option A): the parameter narrows to + * `ExpressionUserSession | null | undefined`; the tightening is compile-time + * only and no runtime behaviour moves. ⛔ B (keep `unknown`) and ⛔ C (a second, + * wider entry point) were declined. + * + * ## Why the instrument is `tsc` and not a test run + * + * Every assertion here is erased before vitest sees it, so a green test run + * proves nothing about any of them. What makes them a pin is + * `packages/app-shell/tsconfig.test.json`, which compiles every + * `src/**\/*.test.ts` in the package and is chained off the package's + * `type-check` script (the script CI's `Type Check` job runs, and + * `scripts/check-type-check-coverage.mjs` enforces the chaining). The package's + * BUILD tsconfig excludes `**\/*.test.ts`, so without that project these + * directives would be read by no compiler at all — failing neither when the + * error they name disappears nor when it was never there, which is + * objectui#3009's failure verbatim. + * + * Two instruments, because they fail in different directions: + * + * - `Assert>` on the parameter type reds if the parameter becomes + * anything other than the declared contract — including `unknown`. + * - `@ts-expect-error` reds via TS2578 ("unused '@ts-expect-error' directive") + * the moment a refusal stops biting. A widened parameter accepts these + * arguments again, the suppressed error goes away, and the directive itself + * becomes the error. + * + * ## Freshness is not the contract + * + * A FRESH object literal is refused by excess-property checking regardless of + * what the parameter declares, so a pin routed only through one measures + * freshness rather than the contract. Every refusal case below is therefore + * routed through a NON-FRESH value — a `const` of a declared type, passed by + * name. `UNCHECKED` in particular is the whole card in one line: an `unknown` + * a caller has not narrowed, which is precisely what all four production call + * sites were free to pass before this change. + * + * ## No build artifact sits between the edit and these assertions + * + * The import below is a relative path to the SOURCE module, and that module is + * a LEAF that imports nothing, so no workspace `dist` can go stale on the path + * and degrade a type to `any`. `AuthUser` comes from the PUBLISHED + * `@objectstack/spec` typings; the `IsAny` guards below fail loudly if either + * import stops resolving, rather than letting a silent `any` turn a refusal + * case green. + */ +import { describe, it, expect } from 'vitest'; +import type { AuthUser } from '@object-ui/auth'; +import type { AuthUser as SpecAuthUser } from '@objectstack/spec/contracts'; + +import { buildExpressionUser, type ExpressionUserSession } from './expressionUser'; + +type Assert = T; +type IsAny = 0 extends 1 & T ? true : false; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; + +/** What the function actually declares it accepts. */ +type Param = Parameters[0]; + +// ── The pin, stated directly on the parameter ─────────────────────────────── +// Guard against the probe lying: were either type `any`, every assertion here +// would pass while proving nothing. +type _ParamNotAny = Assert, false>>; +type _SessionNotAny = Assert, false>>; +type _AuthUserNotAny = Assert, false>>; +type _SpecAuthUserNotAny = Assert, false>>; + +/** + * The card, as one type equation. `unknown` — the shape this parameter carried + * before objectui#6559 — is not equal to the contract, so re-widening reds + * HERE first, with the parameter named in the diagnostic. + */ +type _ParamIsTheDeclaredContract = Assert< + Equal +>; +type _ParamIsNoLongerUnknown = Assert, false>>; + +/** + * The narrowing did not overshoot. Every production call site passes + * `useAuth().user`, typed `AuthUser | null` by `@object-ui/auth`, and the + * signed-out branch is reached by passing `null`. Without these, "reds when the + * parameter widens" would also be satisfied by a parameter that refuses + * everything, which is the other way to disagree with the contract. + * + * ⚠️ The control is `@object-ui/auth`'s `AuthUser`, NOT the spec's, and the + * difference is load-bearing rather than a spelling choice. The spec's + * `AuthUser` is an `interface` with no index signature, and TypeScript infers + * an implicit index signature for type ALIASES only — never for interfaces — so + * the bare spec principal is not assignable to a contract that declares + * `[key: string]: unknown`. `@object-ui/auth`'s `AuthUser` extends it and adds + * that index signature (better-auth projects an app's custom user columns onto + * the object), which is exactly why the four production call sites type + * cleanly. Measured: using the spec interface here produced + * `TS2345: Argument of type 'AuthUser' is not assignable`, which would have + * been a false alarm about the parameter and is really a fact about interfaces. + */ +type _BrowserPrincipalIsAcceptedInput = Assert>; +type _NullIsAcceptedInput = Assert>; +type _UndefinedIsAcceptedInput = Assert>; +type _UncheckedIsNotAcceptedInput = Assert>; + +/** + * The refusal cases as a CALLER writes them, all routed through non-fresh + * values. Assignability above is the contract; a call expression is the + * authoring surface, and excess-property freshness makes the two differ often + * enough to pin both. + */ +const UNCHECKED: unknown = { id: 'u_1', name: 'Ada', email: 'a@e.d' }; +const NO_ID: { name: string; email: string } = { name: 'B', email: 'b@c.d' }; +const OPTIONAL_ID: { id?: string; name: string; email: string } = { + id: 'u_1', + name: 'B', + email: 'b@c.d', +}; +const PRINCIPAL: AuthUser = { id: 'u_1', name: 'Ada', email: 'a@e.d' }; +/** The signed-out arm the same call sites pass — `useAuth().user` is nullable. */ +const NOBODY: AuthUser | null = null; + +describe('objectui#6559 — the parameter is the session contract, checked at every call site', () => { + it('refuses an input the caller has not narrowed', () => { + // THE CARD. Before this change the parameter was `unknown` and this + // compiled — which is why all four production call sites satisfied + // objectui#6551's declaration vacuously. `UNCHECKED` is a const, not a + // literal, so this is the parameter refusing it and not freshness. + // @ts-expect-error objectui#6559 — `unknown` is not a signed-in session. + const fromUnchecked = buildExpressionUser(UNCHECKED); + + // The runtime is unmoved by the narrowing (the ruling: compile-time only), + // so the refused input still normalises exactly as it always did. + expect(fromUnchecked).toMatchObject({ id: 'u_1', name: 'Ada', email: 'a@e.d' }); + }); + + it('refuses a session missing a key the contract declares required', () => { + // Non-fresh, and missing `id` — the exact input objectui#6551 declared + // illegitimate and could not refuse. `name`/`email` are pinned by the type + // equation above; this is the call-expression half. + // @ts-expect-error objectui#6559 — a session with no `id` is not signed in. + const built = buildExpressionUser(NO_ID); + + // ⛔ NOT a fallback. `id: u.id ?? null` is the REJECTED shape (triage + // ruling 2026-08-26, carried into objectui#6559's dispatch): a lenient + // default in the CONSUMER is what AGENTS.md #0.1 forbids, and it silently + // equates "signed in, no id" with "signed out". The honest answer is that + // the type refuses the input; nothing was added below it to paper over one. + expect(built.id).toBeUndefined(); + expect(built.id).not.toBeNull(); + }); + + it('refuses a session whose `id` is merely optional', () => { + // The producer-side spelling of the same defect: a caller whose own type + // makes `id` optional cannot satisfy the contract even when the value is + // present, because the CONTRACT is what the compiler checks, not the value. + // @ts-expect-error objectui#6559 — `id?: string` is wider than the contract. + const built = buildExpressionUser(OPTIONAL_ID); + + expect(built.id).toBe('u_1'); + }); + + it('accepts what the production call sites actually pass', () => { + // The overshoot control, on the real production input type. All four call + // sites pass `useAuth().user`, typed `AuthUser | null`; both arms are here. + const signedIn = buildExpressionUser(PRINCIPAL); + const signedOut = buildExpressionUser(NOBODY); + + expect(signedIn.id).toBe('u_1'); + expect(signedOut.id).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/providers/expressionUser.sessionContract.types.test.ts b/packages/app-shell/src/providers/expressionUser.sessionContract.types.test.ts index e09ebe96bf..8aacd74fa5 100644 --- a/packages/app-shell/src/providers/expressionUser.sessionContract.types.test.ts +++ b/packages/app-shell/src/providers/expressionUser.sessionContract.types.test.ts @@ -56,6 +56,16 @@ * three of the cases below are in that position deliberately, because "the * narrowing did not overshoot" is a claim that needs its own controls. * + * ## The limit this file recorded is closed (objectui#6559) + * + * The last case below used to be ACCEPTED, pinning that the parameter was + * `unknown` and so the narrowed cast bound no call site. objectui#6559 moved + * the declaration onto the parameter (maintainer ruling 2026-08-27), and that + * case is now a REJECTION. The parameter's own discrimination — the leg that + * shows a re-widened parameter turning it green again — lives in + * `expressionUser.parameterContract.types.test.ts`, because the reverted + * preamble here can only re-widen the local type alias. + * * ## Fenced boundary * * `id: u.id ?? null` is the REJECTED shape (triage ruling, 2026-08-26): a @@ -69,7 +79,7 @@ import ts from 'typescript'; import { fileURLToPath } from 'node:url'; import { dirname, join } from 'node:path'; -import { buildExpressionUser } from './expressionUser'; +import { buildExpressionUser, type ExpressionUserSession } from './expressionUser'; const HERE = dirname(fileURLToPath(import.meta.url)); const MODULE_IMPORT = join(HERE, 'expressionUser').replace(/\\/g, '/'); @@ -154,13 +164,18 @@ const CASES: readonly Case[] = [ code: `const i: ExpressionUserSession = { id: 'u_1', name: 'Ada', email: 'a@e.d', role: 'user', positions: ['user'], isPlatformAdmin: true, department__c: 'sales' }; void i;`, }, { - // The honest limit of this change, pinned so no reader has to infer it: the - // PARAMETER is `unknown`, so narrowing the CAST does not make any producer - // loud at compile time. It makes the module stop declaring the broken input - // legitimate. Widening the parameter is a different question and a - // different card. - what: 'the exported function still accepts an unchecked input — the cast is not a parameter check', - rejected: false, + // objectui#6551's honest limit — and objectui#6559 CLOSED it. While the + // PARAMETER was `unknown`, narrowing the cast made no producer loud at + // compile time: the module merely stopped declaring the broken input + // legitimate, and this case recorded that by being ACCEPTED. The parameter + // now IS `ExpressionUserSession | null | undefined` (maintainer ruling + // 2026-08-27, option A), so the same call is refused and this case is the + // seam between the two cards. It is rejected on BOTH legs below: the + // reverted preamble re-widens the local type ALIAS, while the parameter + // reads the real module's. What discriminates the parameter is + // `expressionUser.parameterContract.types.test.ts`. + what: 'the exported function refuses an unchecked input — the parameter is a call-site check', + rejected: true, code: `buildExpressionUser({ name: 'B', email: 'b@c.d' });`, }, ]; @@ -278,8 +293,11 @@ describe('objectui#6551 — the signed-in cast is no wider than the contract it * not move. It pins that the anonymous answer is not an input, * which is a different claim from this card's. * 5 — the spec-derived incomplete principal. - * 6-9 — the overshoot controls and the `unknown`-parameter limit; green on - * both legs by construction. + * 6-8 — the overshoot controls; green on both legs by construction. + * 9 — NOT here either, and for the OPPOSITE reason: since objectui#6559 + * the parameter carries the contract, so that call is refused on + * both legs. This leg re-widens the local type alias only, and the + * parameter reads the real module's declaration. */ const EXPECTED_FLIPS = [0, 1, 2, 3, 5]; @@ -330,7 +348,14 @@ describe('objectui#6551 — the runtime the narrowed declaration now describes', // input never arrives; the type is where that is now said, and this asserts // nothing was quietly added below it. Re-deciding this is a card, not a // test edit. - const built = buildExpressionUser({ name: 'B', email: 'b@c.d' } as unknown); + // Since objectui#6559 the PARAMETER refuses this input, so reaching the + // runtime with it takes a deliberate double assertion — which is the + // honest spelling: no producer can arrive here by accident any more, and + // this case is only asking what the code does if one is forced through. + const built = buildExpressionUser({ + name: 'B', + email: 'b@c.d', + } as unknown as ExpressionUserSession); expect(built.id).toBeUndefined(); expect(built.id).not.toBeNull(); diff --git a/packages/app-shell/src/providers/expressionUser.ts b/packages/app-shell/src/providers/expressionUser.ts index b150bb9852..13ac3e3c8d 100644 --- a/packages/app-shell/src/providers/expressionUser.ts +++ b/packages/app-shell/src/providers/expressionUser.ts @@ -90,12 +90,34 @@ * ("the shape that teaches the wrong thing") and the one objectui#6534 refused * for the anonymous branch, one key over. * - * The narrowing is DECLARATIVE, and deliberately only that. Nothing reachable - * today hands this function a session without `id` — the better-auth principal - * above always carries one — so no runtime behaviour moves here and none was - * made to move: the defect was that the declaration LIED about the contract, - * not that a user could reach it. What refuses the widening from coming back is - * the compiler, via `expressionUser.sessionContract.types.test.ts`. + * Nothing reachable today hands this function a session without `id` — the + * better-auth principal above always carries one — so no runtime behaviour + * moves here and none was made to move: the defect was that the declaration + * LIED about the contract, not that a user could reach it. What refuses the + * widening from coming back is the compiler, via + * `expressionUser.sessionContract.types.test.ts`. + * + * ## This type is the PARAMETER, not a cast (objectui#6559) + * + * objectui#6551 wrote the shape above but left the parameter `unknown`, with + * the narrowing applied INSIDE as `user as ExpressionUserSession | …`. A cast + * binds nothing: every call site satisfied the declaration vacuously, and + * `buildExpressionUser({ name: 'B', email: 'b@c.d' })` still compiled. A + * declaration nothing checks is indistinguishable from no declaration at all + * (AGENTS.md #0.1), which is why the shape above and the signature below are + * now the SAME statement rather than two that merely agree. + * + * Maintainer ruling 2026-08-27 (option A): the parameter narrows to + * `ExpressionUserSession | null | undefined`. It is a tightening of a + * package-entry export, so an external caller passing an unchecked value gets a + * compile error on upgrade — accepted, because such a call was never + * contract-conformant. All four in-repo production call sites pass + * `useAuth().user` (`AuthUser | null`) and type cleanly, measured: two in + * `console/AppContent.tsx`, one in `views/RecordFormPage.tsx`, one in + * `apps/console`'s `InternalFormRoute.tsx`. Runtime output is unchanged for + * every input any producer can supply. ⛔ B (keep `unknown`) and ⛔ C (a second, + * wider entry point) were declined. The pin is + * `expressionUser.parameterContract.types.test.ts`. * * ⛔ Not a fallback. `id: u.id ?? null` was the rejected shape (triage ruling, * 2026-08-26): it puts a lenient default in the CONSUMER, which AGENTS.md #0.1 @@ -119,9 +141,10 @@ export type ExpressionUserSession = { [key: string]: unknown; }; -export function buildExpressionUser(user: unknown): Record { - const u = user as ExpressionUserSession | null | undefined; - if (!u) { +export function buildExpressionUser( + user: ExpressionUserSession | null | undefined, +): Record { + if (!user) { return { // `null`, not absent — objectui#6534. An ABSENT key is not `false`: it // makes `ctx.user.id == '…'` FAULT, and a faulting visibility predicate @@ -141,21 +164,21 @@ export function buildExpressionUser(user: unknown): Record { }; } return { - id: u.id, - name: u.name, - email: u.email, - role: u.role ?? 'user', + id: user.id, + name: user.name, + email: user.email, + role: user.role ?? 'user', // Surface the platform-admin flag so action `visible` CEL predicates // gated on `ctx.user.isPlatformAdmin == true` (e.g. sys_environment // "Change Plan (admin)") evaluate correctly. Previously only // name/email/role were forwarded → isPlatformAdmin-gated actions were // hidden even for platform admins. - isPlatformAdmin: u.isPlatformAdmin ?? false, + isPlatformAdmin: user.isPlatformAdmin ?? false, // Positions are what the SERVER binds as `current_user` for per-option // `visibleWhen` authorization gating (ADR-0058; framework EvalUser — // objectui#2284). Forwarding them lets a position-gated option // (`'admin' in current_user.positions`) hide client-side too, instead // of failing open as visible and only being rejected on submit. - positions: u.positions ?? [], + positions: user.positions ?? [], }; }