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
46 changes: 46 additions & 0 deletions .changeset/16875-nav-recordid-viewname-tolerated.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
'@objectstack/spec': patch
---

`ObjectNavItem.recordId`'s docblock said it was "Mutually exclusive with `viewName`" — the guard tolerates that exact pair, deliberately

The docblock read *"Mutually exclusive with `viewName` (viewName is ignored if
both are set)"*. The parenthetical was the tell: *"ignored if both are set"*
describes a **precedence**, not a refusal, so the sentence's own second clause
contradicted its first — and the code agrees with the second clause.
`recordId` + `viewName` parses clean through `NavigationItemSchema`; it is the
one legacy combination `objectNavTargetExclusivity` lets through, and that
guard's own docblock says so in as many words.

**The harm direction is silent in both directions.** An author (or an agent)
who read "mutually exclusive" would avoid a combination the platform accepts,
or file a bug when it parses. Two docblocks in one file described one rule and
disagreed; the guard's was right.

⛔ **No behaviour changes, and the asymmetry is not "unified".** The tolerance
is a recorded decision, and `app-nav-target-exclusivity-export.test.ts` already
pins `recordId` + `viewName` as accepted precisely so that making the target
fields pairwise exclusive goes red. This changeset corrects the **prose** only:
no schema, no guard, no accept set, no authorable key, no export moves. The
`.describe()` strings — the ones that reach `content/docs/references/` — are
untouched.

The corrected docblock now says the pair is tolerated rather than refused,
names the guard that tolerates it, and points at the test that pins it. The
same test file gains a fifth leg asserting the docblock against the accept set
it describes, so the next copy of this sentence goes red instead of shipping:
prose is the only place the tolerated pair is documented, so nothing else was
watching it.

**This is shipped, which is why it carries a changeset rather than
`skip-changeset`.** `@objectstack/spec`'s published `files[]` carries both
`dist` and `src/**/*.zod.ts`, and `src/ui/app.zod.ts` matches that glob — the
edited file is shipped as source verbatim. Measured on the built artifact as
well: the new sentence is present in **18** built files under `dist/` and the
old spelling in **0**, with two untouched sentences from the same region
(`navigate straight to the detail page`, and the `filters` docblock's own TRUE
exclusivity claim over `recordId` / `viewName`) present in **18** each as the
lit controls, so the zero is a reading and not a mistyped anchor. The
declaration files do not carry it — this is a field-level docblock inside a Zod
shape — which is why the reach is stated as the bundles and the shipped source
rather than as `.d.ts`.
48 changes: 48 additions & 0 deletions packages/spec/src/ui/app-nav-target-exclusivity-export.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,51 @@ describe('`./index` (the `@objectstack/spec/ui` surface) exports the same functi
expect(objectNavTargetExclusivity.length).toBe(2);
});
});

// ---------------------------------------------------------------------------
// Leg 5 — the `recordId` docblock agrees with the accept set it describes
// ---------------------------------------------------------------------------

/**
* The tolerated pair is the one an author learns about from PROSE, not from a
* refusal: nothing goes red when the docblock drifts away from it. The
* `recordId` docblock did drift — it declared the pair "mutually exclusive"
* while leg 1 pins it as accepted — so the prose gets a pin of its own beside
* the measurement it has to match.
*
* The extraction throws when its anchor moves, so this can never pass
* vacuously on a docblock it failed to find. The docblock is normalised the
* way a JSDoc block has to be before it can be searched: strip the leading
* `*` of each line first, THEN flatten whitespace — a claim wraps lines, and
* a raw search reads a confident 0 on text that is plainly there.
*/
describe('`recordId` docblock parity with the accept set (#16875)', () => {
const src = fs.readFileSync(path.join(HERE, 'app.zod.ts'), 'utf8');

const recordIdDocblock = ((): string => {
const m = src.match(/\/\*\*([\s\S]*?)\*\/\s*\n\s*recordId: z\.string\(\)\.optional\(\)/);
if (!m) throw new Error('recordId docblock not found in app.zod.ts — the anchor moved, re-locate it BY TEXT');
return m[1].replace(/^[ \t]*\*[ \t]?/gm, '').replace(/\s+/g, ' ').trim();
})();

it('the extraction is lit — it found a non-empty docblock that names `viewName`', () => {
expect(recordIdDocblock.length).toBeGreaterThan(80);
expect(recordIdDocblock).toContain('viewName');
});

it('claims no exclusivity the guard does not enforce', () => {
expect(recordIdDocblock).not.toMatch(/mutually exclusive/i);
expect(recordIdDocblock).not.toMatch(/not combinable/i);
expect(recordIdDocblock).not.toMatch(/cannot be combined/i);
});

it('says the pair is tolerated — and the schema still accepts it', () => {
expect(recordIdDocblock).toMatch(/tolerated/i);
const r = NavigationItemSchema.safeParse({
...NAV,
recordId: '{current_user_id}',
viewName: 'all',
});
expect(r.success).toBe(true);
});
});
12 changes: 10 additions & 2 deletions packages/spec/src/ui/app.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,16 @@ export const ObjectNavItemSchema = lazySchema(() => strictObject(navItemSurface(
* When set, navigate straight to the detail page of this specific
* record instead of the object's list view. Supports template
* variables `{current_user_id}` and `{current_org_id}` resolved by
* the shell at render time. Mutually exclusive with `viewName`
* (viewName is ignored if both are set).
* the shell at render time.
*
* Combining it with `viewName` is TOLERATED, not refused — it is the one
* legacy pair {@link objectNavTargetExclusivity} deliberately lets through
* (that guard's own docblock names it), and `viewName` is ignored when both
* are set. Read as an exclusivity, this sentence would send an author away
* from a combination the platform accepts, or have them file a bug when it
* parses. The tolerance is deliberate and pinned by
* `app-nav-target-exclusivity-export.test.ts`; the combinations that ARE
* refused live on `filters` and `runAction`.
*/
recordId: z.string().optional().describe(
'Navigate directly to this record id instead of the list view. Supports template vars: {current_user_id}, {current_org_id}.',
Expand Down
Loading