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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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
39 changes: 39 additions & 0 deletions .changeset/lint-field-rule-ambient-roots.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/lint": patch
---

fix(lint): a field-level `*When` reading `app` gets the scope diagnostic, not the false `record.app` prescription (#13935)

`fieldRuleRootIssue` judged field-rule roots against `@objectstack/formula`'s
`SCOPE_ROOTS`, which answers "is this root declared **platform-wide**". The
question this rule needs answered is "is this root bound at **some** evaluation
site". The two agreed for all 27 baseline roots and disagreed for exactly one:
`app`, which objectui's `ExpressionProvider` binds on the form-view surface an
author migrates a field rule *down* from.

Falling outside the membership test sent `app` to the generic bare-reference
check, whose prescription is ``Write `record.app` `` — and following that
advice earns ``unknown field `app` on `invoice` `` from the field-existence
pass. A first diagnostic that asserts something false about where the root
binds, plus a wasted correction cycle. `current_user`, `user`, `ctx`, `os`,
`features` and `data` all got the correct message; `app` alone did not.

Authoring a field-level `visibleWhen` / `readonlyWhen` / `requiredWhen` on
`app` now earns the same scope diagnostic every other unbound root gets —
"a field-level conditional rule binds only `record` (plus `previous`, and
`parent` on a master-detail line item)" — with a prescription tier of its own
that says what is actually true of an ambient root: it is *not* declared
platform-wide, it is mounted only by the renderer, and `record.app` is
explicitly refused rather than merely omitted, because that is the advice the
author just followed out of the old diagnostic.

**No accept set moves.** `SCOPE_ROOTS` is `@objectstack/formula`'s published
strict-lint baseline — adding `app` there would stop *every* surface that
judges bare identifiers from faulting it, to fix one surface's wording. The
widened vocabulary is assembled in `@objectstack/lint` instead, where the
per-surface question is asked, and both diagnostics involved were already
`severity: 'error'`, so this changes which message an author reads and nothing
about what lints clean.

`FIELD_RULE_AMBIENT_ROOTS` and `FIELD_RULE_JUDGED_ROOTS` are exported beside
the existing `FIELD_RULE_BOUND_ROOTS`.
193 changes: 188 additions & 5 deletions packages/lint/src/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import { ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
import { SharingRuleSchema } from '@objectstack/spec/security';

import { validateStackExpressions, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js';
import {
validateStackExpressions,
FIELD_RULE_BOUND_ROOTS,
FIELD_RULE_AMBIENT_ROOTS,
FIELD_RULE_JUDGED_ROOTS,
} from './validate-expressions.js';
import type { ExprIssue } from './validate-expressions.js';
// [#8405] Cross-site pin only — see the describe block at the bottom of this
// file. Not otherwise used here; validate-semantic-roles.test.ts owns the
Expand DownExpand Up@@ -956,15 +961,19 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
* resolves in the strict env, so the bare-reference check never fired on it
* either, and the denylist did not know it.
*
* These tests are written against the IMPORTED `SCOPE_ROOTS`, not a copy of
* These tests are written against the IMPORTED vocabulary, not a copy of
* it, because "a future root is covered for free" is the whole argument for
* the allowlist and a hand-copied list in the test would assert the opposite
* of what it claims — it would go green on a root the rule never saw.
* of what it claims — it would go green on a root the rule never saw. Since
* #13935 the imported thing is `FIELD_RULE_JUDGED_ROOTS` rather than
* `SCOPE_ROOTS`: the judged vocabulary is now the WIDER "bound at some
* evaluation site" set, and generating from the baseline would have left
* exactly the ambient roots #13935 added out of the table.
*/
describe('field-level `*When` roots are an ALLOWLIST over SCOPE_ROOTS (#6713)', () => {
describe('field-level `*When` roots are an ALLOWLIST over the judged vocabulary (#6713/#13935)', () => {
/** The three the surface really binds. Everything else must be rejected. */
const BOUND = ['record', 'previous', 'parent'] as const;
const RESIDUAL = SCOPE_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));
const RESIDUAL = FIELD_RULE_JUDGED_ROOTS.filter((r) => !(BOUND as readonly string[]).includes(r));

const fieldIssues = (predicate: string, slot = 'visibleWhen') =>
validateStackExpressions({
Expand DownExpand Up@@ -1019,6 +1028,171 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
expect(hit[0]!.message).toContain('`visibleWhen` reads `data`');
});

/**
* ── The AMBIENT roots (#13935) ────────────────────────────────────────
*
* `SCOPE_ROOTS` answers "is this declared platform-wide"; this rule needs
* "is this bound at SOME evaluation site". They agreed for 27 roots and
* disagreed for `app`, which objectui's `ExpressionProvider` binds on the
* very surface an author migrates a rule DOWN from. Falling outside the
* membership test sent `app` to the bare-reference check, which
* prescribed `record.app` — and following THAT earns `unknown field
* \`app\``. The defect is WHICH diagnostic fires, so every assertion here
* names the specific diagnostic rather than counting that "something
* fired".
*/
describe('ambient roots — bound somewhere, absent from SCOPE_ROOTS (#13935)', () => {
/**
* The ruling, pinned as a boundary test rather than restated in prose:
* the fix widens the vocabulary THIS package assembles and leaves
* `@objectstack/formula`'s published accept baseline alone. A future
* edit that "simplifies" this by adding `app` to `SCOPE_ROOTS` widens a
* published accept set — every surface judging bare identifiers stops
* faulting it — and goes red right here.
*/
it('does NOT widen `SCOPE_ROOTS` — the judged set is a strict superset assembled locally', () => {
expect([...FIELD_RULE_AMBIENT_ROOTS]).toEqual(['app']);
// The baseline is untouched: `app` is still not declared platform-wide.
expect(SCOPE_ROOTS).not.toContain('app');
// …and the judged vocabulary contains all of it, plus the ambient set.
expect(FIELD_RULE_JUDGED_ROOTS).toEqual([...SCOPE_ROOTS, ...FIELD_RULE_AMBIENT_ROOTS]);
expect(FIELD_RULE_JUDGED_ROOTS.length).toBe(SCOPE_ROOTS.length + 1);
});

it('gives `app` the SCOPE diagnostic — not the bare-reference prescription', () => {
const hit = fieldIssues("app.locale == 'en'");
// One verdict, not two: the bare-reference check no longer also fires.
// ⛔ Do not soften this to `toBeGreaterThan(0)` — the length IS the
// pin that catches the suppression silently missing.
expect(hit).toHaveLength(1);
expect(hit[0]!.severity).toBe('error');
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
expect(hit[0]!.message).toContain('binds only `record`');
// The wording the card was filed about, in both halves: the generic
// diagnostic's identity, and the prescription that is actively false.
expect(hit[0]!.message).not.toContain('bare reference');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

it('tells `app` the truth about where it binds — ambient, renderer-only', () => {
const hit = fieldIssues("app.locale == 'en'");
expect(hit[0]!.message).toContain('AMBIENT root');
expect(hit[0]!.message).toContain('⛔ Do NOT write `record.app`');
// ⛔ NOT the general tier's claim, which is false for an ambient root
// in both of its clauses.
expect(hit[0]!.message).not.toContain('is declared platform-wide');
});

/**
* Why `record.app` had to be refused IN the message rather than merely
* left out: it is exactly what the pre-#13935 diagnostic told this
* author to write, and it does not work.
*/
it('pins that the OLD prescription was false — `record.app` earns `unknown field`', () => {
const hit = fieldIssues('record.app == 1');
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('unknown field `app`');
});

/**
* The discriminator. `current_user` is the positive control that passed
* before this card and must keep passing UNCHANGED — same tier, same
* prescription. A repair that gave every rejected root the new ambient
* wording would satisfy the `app` assertions above and be wrong.
*/
it('leaves the `current_user` control on the USER tier, not the ambient one', () => {
const hit = fieldIssues("current_user.id == 'U1'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `current_user`');
expect(hit[0]!.message).toContain('move the predicate to the option\'s own');
expect(hit[0]!.message).not.toContain('AMBIENT root');
});

/**
* Tie-break no-regression. `SCOPE_ROOTS` is spliced in FIRST, so a
* predicate reading both a baseline root and an ambient one reports the
* baseline root — the same root, and the same message, it reported
* before #13935 widened the vocabulary.
*
* The LENGTH is the second half of this pin and it is the half that
* moved: before #13935 this predicate earned two issues — the `ctx`
* verdict plus a bare reference to `app` prescribing `record.app`, the
* exact false advice this card removes. The rule emits one verdict per
* slot, so `app` waits its turn rather than being told something untrue.
*/
it('keeps the pre-#13935 tie-break — a baseline root still wins over an ambient one', () => {
const hit = fieldIssues("ctx.locale == 'en' && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `ctx`');
expect(hit[0]!.message).not.toContain('Write `record.app`');
});

/**
* …and the second root is not LOST, only deferred: fixing `ctx` earns
* `app` its own correct verdict on the next run. Without this the pin
* above would be satisfied by a repair that simply dropped the root.
*/
it('reports the ambient root on the next pass, once the baseline root is fixed', () => {
const hit = fieldIssues("record.amount > 0 && app.locale == 'en'");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('`visibleWhen` reads `app`');
});

/**
* Root-vs-MEMBER, at ambient width: `record.app_id` is an ordinary
* field name that merely starts like the new root.
*/
it('does NOT trip on a `record` member merely spelled like an ambient root', () => {
const issues = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
app_id: { type: 'text' },
gate: { type: 'text', visibleWhen: "record.app_id != ''" },
},
}],
}).filter((i) => i.where.includes("field 'gate' visibleWhen"));
expect(issues).toHaveLength(0);
});

/**
* BLAST RADIUS. The suppression is gated on a field-rule verdict, so it
* reaches the field level and nothing else. A per-OPTION `visibleWhen`
* is deliberately NOT passed through this rule (options resolve against
* the host's predicate scope — see the #6290 note in the field walk),
* so `app` there still meets the bare-reference check exactly as it did
* before this card. Pinned because "suppress the bare-reference verdict"
* is the half of this repair that could quietly go wide.
*/
it('does not reach the per-OPTION surface — `app` there keeps the bare-reference verdict', () => {
const hit = validateStackExpressions({
objects: [{
name: 'showcase_deal',
fields: {
gate: {
type: 'select',
options: [{ value: 'a', visibleWhen: "app.locale == 'en'" }],
},
},
}],
}).filter((i) => i.where.includes('option'));
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `app`');
});

/**
* …and a bare FIELD reference on a field-rule slot is untouched: no
* ambient root is read, so no verdict fires and nothing is suppressed.
* Guards the gate itself — a suppression keyed on the wrong condition
* would swallow this and leave the author with silence.
*/
it('leaves a plain bare field reference on a field-rule slot alone', () => {
const hit = fieldIssues("nope == 1");
expect(hit).toHaveLength(1);
expect(hit[0]!.message).toContain('bare reference `nope`');
});
});

/**
* The partition, both halves. The rule judges `SCOPE_ROOTS` membership,
* NOT strict-env declaredness — and that is a measured distinction, not a
Expand DownExpand Up@@ -2264,6 +2438,15 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
// receiver above) and the provenance index (`unprovisionedIndex` /
// `anchors`), whose keys are Map/Set methods, never metadata keys.
'pending', 'celNode', 'celRecv', 'anchors', 'unprovisionedIndex',
// [#13935] The field-rule verdict, split into a compute half and a push
// half so the walk can tell `check` a verdict was issued. `verdict`'s
// keys are this helper's own `{ root, message, source }`, never metadata
// keys; `diagnostic` is a formula error STRING and its one "key" is
// `String.prototype.startsWith`. Both are named to stay clear of the
// `message` / `source` metadata receivers — a local called `message`
// here would have been excused into masking a genuine
// `validations[].message` read.
'verdict', 'diagnostic',
]);
expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
});
Expand Down
Loading
Loading