Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/action-newtaburl-requires-opensinnewtab.md
Original file line numberDiff line numberDiff line change
@@ -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.

<!-- adr-0087: not-required (no-migration-prescription) A validity narrowing over a pair of existing keys: no key is removed, renamed or re-shaped, so there is no tombstone and nothing mechanical for `objectstack migrate meta` to rewrite. The refusal is the channel that reaches an affected author, at the parse site, carrying both remedies; whether a lone `newTabUrl` meant "add the flag" or "delete the leftover" is authoring intent no migration entry can decide on an upgrader's behalf — and the measured population of affected sources is zero in every corpus. Mirrors the disposition of the adjacent #11519 narrowing on the same schema. -->
140 changes: 140 additions & 0 deletions packages/spec/src/ui/action-newtaburl-pair.test.ts
Original file line numberDiff line numberDiff line change
@@ -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<string, unknown>;
// 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<string, unknown>;
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);
});
});
});
33 changes: 33 additions & 0 deletions packages/spec/src/ui/action.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<typeof ActionSchema>;
Expand Down
Loading