diff --git a/.changeset/action-newtaburl-requires-opensinnewtab.md b/.changeset/action-newtaburl-requires-opensinnewtab.md new file mode 100644 index 0000000000..fb90c2e57b --- /dev/null +++ b/.changeset/action-newtaburl-requires-opensinnewtab.md @@ -0,0 +1,36 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): enforce the documented `newTabUrl` / `opensInNewTab` co-constraint on `ActionSchema` (#11842) + +**BREAKING** accept-set narrowing on `ActionSchema`, shipped as `minor` under +the repo's launch-window convention for breaking changes. + +`newTabUrl`'s doc has always said "Only valid together with `opensInNewTab`", +and every renderer read point agrees: objectui's pre-opened-tab wrapper reads +the key only behind `action.opensInNewTab && newTabUrl`, and no other path +reads it at all. Nothing on the refine chain enforced the pairing, so an +action declaring `newTabUrl` without `opensInNewTab: true` parsed clean and +the key was silently inert — the ADR-0078 declared-but-unenforced shape, +arriving through a documented co-constraint rather than a missing key. + +`ActionSchema` now **rejects at parse time** an action declaring `newTabUrl` +whose `opensInNewTab` is not `true`, with guidance naming the pre-opened-tab +contract and both remedies (declare the flag if a pre-opened tab is intended; +otherwise delete the inert key — behavior is unchanged either way it was +already behaving, because the lone key was never read). An explicit +`opensInNewTab: false` beside `newTabUrl` is refused too, deliberately: +unlike the #11519 doubled-channel rule, `newTabUrl` has no meaning outside +the pre-opened-tab flow, so a declared-off channel leaves the key exactly as +dead as an undeclared one. + +The legal pairing is untouched and pinned byte-identically: `opensInNewTab: +true` + `newTabUrl`, `opensInNewTab` alone, and `opensInNewTab: false` alone +all parse exactly as before. The corpus was measured at zero lone-`newTabUrl` +producers (this repo's examples and platform metadata, objectui's fixtures +and renderer read points, and the cloud SSO producers, which declare the pair +correctly — re-measured at claim per the triage requirement), so no shipped +metadata is affected. + + diff --git a/packages/spec/src/ui/action-newtaburl-pair.test.ts b/packages/spec/src/ui/action-newtaburl-pair.test.ts new file mode 100644 index 0000000000..ed0306d5a5 --- /dev/null +++ b/packages/spec/src/ui/action-newtaburl-pair.test.ts @@ -0,0 +1,140 @@ +// #11842 — the `newTabUrl` / `opensInNewTab` co-constraint, enforced. +// +// `newTabUrl`'s doc has always said "Only valid together with `opensInNewTab`", +// and every renderer read point agrees (objectui's pre-opened-tab wrapper reads +// the key only behind `action.opensInNewTab && newTabUrl`; nothing else reads +// it). Before the refine, an action declaring `newTabUrl` WITHOUT +// `opensInNewTab: true` parsed clean and the key was silently inert — the +// ADR-0078 declared-but-unenforced shape, arriving through a documented +// co-constraint rather than a missing key. The refine turns the lone key into +// an authoring-time refusal whose message names the pre-opened-tab contract +// and both remedies. +// +// Scope notes pinned below, because they are deliberate and OPPOSITE to the +// #11519 rule beside this one: there, `opensInNewTab: false` is NOT the marker +// (a declared-off channel means the pair carries one destination and stays +// accepted); here, `opensInNewTab: false` beside `newTabUrl` IS refused — the +// key has no meaning outside the pre-opened-tab flow, so a declared-off +// channel leaves it exactly as dead as an undeclared one. And the rule is +// type-independent: no action type reads a lone `newTabUrl`. +import { describe, it, expect } from 'vitest'; +import { ActionSchema } from './action.zod'; +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; + +const base = { name: 'open_sso_portal', label: 'Open SSO portal' }; + +describe('ActionSchema — newTabUrl requires opensInNewTab: true (#11842)', () => { + describe('refusal pins — the lone key, on every shape that can carry it', () => { + const lone = { + ...base, + type: 'script' as const, + target: 'ssoOpen', + newTabUrl: '/sso-open?recordId={recordId}', + }; + + it('refuses newTabUrl without opensInNewTab', () => { + const r = ActionSchema.safeParse(lone); + expect(r.success).toBe(false); + }); + + it('locates the issue on the newTabUrl path and names the contract and both remedies', () => { + const r = ActionSchema.safeParse(lone); + expect(r.success).toBe(false); + const issues = r.error!.issues; + // The refusal is located on the offending key, not on the object root. + expect(issues.some((i) => i.path.join('.') === 'newTabUrl')).toBe(true); + const msg = issues.map((i) => i.message).join('\n'); + // Both fields of the pair, by name, and the flag's required value. + expect(msg).toContain('newTabUrl'); + expect(msg).toContain('opensInNewTab: true'); + // The pre-opened-tab contract the doc text carries (:1304 describe). + expect(msg).toMatch(/pre-open/i); + // Both remedies: add the flag, or drop the inert key. + expect(msg).toMatch(/add\s+`opensInNewTab: true`/); + expect(msg).toMatch(/drop\s+`newTabUrl`/); + // The silently-inert class this file rejects at author time. + expect(msg).toContain('ADR-0078'); + }); + + it('refuses newTabUrl beside an explicit opensInNewTab: false — a declared-off channel leaves the key just as dead', () => { + const r = ActionSchema.safeParse({ + ...base, + type: 'script', + target: 'ssoOpen', + opensInNewTab: false, + newTabUrl: '/sso-open?recordId={recordId}', + }); + expect(r.success).toBe(false); + expect(r.error!.issues.some((i) => i.path.join('.') === 'newTabUrl')).toBe(true); + }); + + it('is type-independent: a lone newTabUrl on type:url and type:api is refused too', () => { + for (const shape of [ + { ...base, type: 'url' as const, target: 'https://example.com', newTabUrl: '/x/{recordId}' }, + { ...base, type: 'api' as const, target: '/api/v1/actions/x/y', newTabUrl: '/x/{recordId}' }, + ]) { + const r = ActionSchema.safeParse(shape); + expect(r.success, JSON.stringify(shape)).toBe(false); + expect(r.error!.issues.some((i) => i.path.join('.') === 'newTabUrl')).toBe(true); + } + }); + + it('is refused through the registered `action` metadata schema too (the parsing door)', () => { + const schema = getMetadataTypeSchema('action'); + expect(schema).toBeDefined(); + const r = schema!.safeParse(lone); + expect(r.success).toBe(false); + }); + }); + + describe('legal-pair pins — the documented pairing stays accepted byte-identically', () => { + it('opensInNewTab: true + newTabUrl on a script action: accepted, output unchanged', () => { + const out = ActionSchema.parse({ + ...base, + type: 'script', + target: 'ssoOpen', + opensInNewTab: true, + newTabUrl: '/sso-open?recordId={recordId}', + }) as Record; + // The exact parse output this input produced BEFORE the refine landed — + // materialized defaults included. A byte drift here means the narrowing + // touched the accepted case. + expect(out).toEqual({ + name: 'open_sso_portal', + label: 'Open SSO portal', + type: 'script', + target: 'ssoOpen', + refreshAfter: false, + opensInNewTab: true, + newTabUrl: '/sso-open?recordId={recordId}', + }); + }); + + it('opensInNewTab alone (handler-redirect channel, no direct URL): accepted, output unchanged', () => { + const out = ActionSchema.parse({ + ...base, + type: 'script', + target: 'ssoOpen', + opensInNewTab: true, + }) as Record; + expect(out).toEqual({ + name: 'open_sso_portal', + label: 'Open SSO portal', + type: 'script', + target: 'ssoOpen', + refreshAfter: false, + opensInNewTab: true, + }); + }); + + it('opensInNewTab: false alone (channel declared off, no newTabUrl): accepted', () => { + const r = ActionSchema.safeParse({ + ...base, + type: 'script', + target: 'cloneVersion', + opensInNewTab: false, + }); + expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true); + }); + }); +}); diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index f37919f5e6..fea685cefd 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -1571,6 +1571,39 @@ export const ActionSchema = lazySchema(() => actionObject().refine((data) => { + '`opensInNewTab` + the handler redirect and drop `onSuccess`. There is no `precedence` ' + 'field, by ruling: one destination, declared in one place.', path: ['onSuccess'], +}).refine((data) => { + // #11842 — enforce the documented co-constraint on `newTabUrl`. The field's + // own doc has always said "Only valid together with `opensInNewTab`", and + // every renderer read point agrees: objectui's pre-opened-tab wrapper + // (`consoleServerAction.ts`) reads `newTabUrl` only behind + // `action.opensInNewTab && newTabUrl`, and no other path reads the key at + // all. So a lone `newTabUrl` parsed clean and did nothing — the + // declared-≠-enforced shape this file rejects at author time (#4352 / + // ADR-0078), arriving through a documented co-constraint rather than a + // missing key. + // + // `opensInNewTab: false` is refused too, deliberately: unlike the #11519 + // rule above (where `false` declares the handler-redirect channel out of + // use and the pair carries ONE destination), `newTabUrl` has no meaning + // OUTSIDE the pre-opened-tab flow — a declared-off channel makes the key + // exactly as dead as an undeclared one. The corpus was measured at zero + // lone producers (this repo's examples/platform metadata, objectui + // fixtures, and the cloud SSO producers all declare the pair), so nothing + // legal breaks. + if (data.newTabUrl !== undefined && data.opensInNewTab !== true) { + return false; + } + return true; +}, { + message: + '`newTabUrl` is the zero-roundtrip target of the pre-opened-tab flow and is only valid ' + + 'together with `opensInNewTab: true` — the renderer pre-opens the tab synchronously on ' + + 'click and only THAT flow ever navigates to `newTabUrl`; no render path reads the key ' + + 'otherwise, so without the flag it would parse clean and never run (ADR-0078). If a ' + + 'pre-opened tab is intended, add `opensInNewTab: true`; otherwise drop `newTabUrl` ' + + '(behavior is unchanged — the lone key was never read). For a STATIC url action, new-tab ' + + 'behavior is `openIn: "new-tab"`, not this pair (#11842).', + path: ['newTabUrl'], }).transform((data, ctx) => lowerRequiresFeature(data, ctx))); export type Action = z.input;