Skip to content

Commit f35cdc5

Browse files
authored
feat(spec): cover all three fold-and-drop aliases, not just execute (#3743) (#3854)
#3838 introduced `lintDeprecatedAliases` — the pre-parse pass that reports an alias the parse itself is about to consume — with one rule, for `action.execute`. #3743 predicted the pass would earn its keep beyond that rule. It does: `execute` was never special. The spec has exactly THREE transforms that fold an alias into its canonical key and then drop it from the parsed output, and all three share one failure mode — declare both slots with different values and one is discarded with no signal, invisible to every downstream check because the parse already erased the evidence. Two more rules, same shape, same advisory severity, same two surfaces: field-requiredwhen-conditionalrequired-conflict — `FieldSchema` folds `conditionalRequired` into `requiredWhen` (#3754/#3764); the discarded predicate never gates the field. Covers fields on objects AND on object extensions, and compares the predicate TEXT so a bare string and the `{ dialect, source }` envelope it lowers into read as the same predicate. agent-knowledge-sources-topics-conflict — `AIKnowledgeSchema` folds `knowledge.topics` into `knowledge.sources` (#1878/#1891); the discarded list names RAG sources the agent never recruits from. Compares by SET, so the same sources in a different order stay quiet. The three findings now share one builder, so they phrase the common half identically (which slot wins, that the alias is dropped, delete it) while each still names what its own discarded value would have done. No CLI change: both call sites already loop over whatever the pass returns. Also corrects `content/docs/ai/agents.mdx`, which documented `knowledge` as `{ topics, indexes }` and used `topics` in all three examples — teaching the deprecated alias as the canonical key, and contradicting `skills/objectstack-ai/SKILL.md`, which already had it right. Follow-ups filed: #3855 (removal schedule for all three aliases), #3856 (objectui's now-dead `execute || target` fallback).
1 parent 8fbbd70 commit f35cdc5

8 files changed

Lines changed: 439 additions & 44 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): the deprecated-alias warning now covers all three fold-and-drop aliases (#3743 follow-up)
6+
7+
#3838 introduced `lintDeprecatedAliases` — the pre-parse pass that reports an
8+
alias the parse is about to consume — with one rule, for `action.execute`. The
9+
issue that asked for it predicted the pass would earn its keep beyond that rule,
10+
and it does: `execute` was never special. The spec has exactly **three**
11+
transforms that fold an alias into its canonical key and then drop it from the
12+
parsed output, and all three share the same failure mode — declare both slots
13+
with different values and one of them is discarded with no signal, invisible to
14+
every downstream check because the parse already erased it.
15+
16+
Two more rules, same shape, same advisory severity, same two surfaces
17+
(`defineStack` at authoring time; `os build` / `os validate` for stacks that skip
18+
strict `defineStack`):
19+
20+
- **`field-requiredwhen-conditionalrequired-conflict`**`FieldSchema` folds
21+
`conditionalRequired` into `requiredWhen` (#3754). The discarded predicate
22+
never gates the field. Covers fields on objects *and* on object extensions.
23+
Compares the predicate **text**, so a bare string and the
24+
`{ dialect, source }` envelope it lowers into are recognised as the same
25+
predicate and stay quiet.
26+
- **`agent-knowledge-sources-topics-conflict`**`AIKnowledgeSchema` folds
27+
`knowledge.topics` into `knowledge.sources` (#1891). The discarded list names
28+
RAG sources the agent never recruits from. Compares by **set**, so the same
29+
sources in a different order stay quiet.
30+
31+
Neither fails the build; both name the two values and give the one-line fix.
32+
33+
Also corrects `content/docs/ai/agents.mdx`, which documented `knowledge` as
34+
`{ topics, indexes }` and used `topics` in all three examples — teaching the
35+
deprecated alias as if it were the canonical key, and disagreeing with
36+
`skills/objectstack-ai/SKILL.md`, which already had it right. The examples now
37+
use `sources`.

content/docs/ai/agents.mdx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ own `ask` / `build` records use exactly these fields):
137137
| `model` | Provider + model config (`provider`: `openai` \| `azure_openai` \| `anthropic` \| `local`) |
138138
| `skills` | Skill names to attach (the primary Agent → Skill → Tool capability model) |
139139
| `tools` | Direct tool **references** `{ type, name, description }``type` is `action` \| `flow` \| `query` \| `vector_search`; `name` points at an existing Action/Flow/query |
140-
| `knowledge` | RAG access: `{ topics: string[], indexes: string[] }` |
140+
| `knowledge` | RAG access: `{ sources: string[], indexes: string[] }`. `sources` is the canonical key the renderer reads; `topics` is a deprecated alias folded into it at parse time |
141141

142142
There is no `type` field and no fixed agent "type" taxonomy — behaviour comes from
143143
persona, instructions, skills, and tools. There are no `triggers` / `schedule`
@@ -199,9 +199,9 @@ Always be professional and data-driven.`,
199199
{ type: 'action', name: 'generate_email', description: 'Generate a personalized email template' },
200200
],
201201

202-
// RAG access: topics to recruit knowledge from + vector store indexes.
202+
// RAG access: sources to recruit knowledge from + vector store indexes.
203203
knowledge: {
204-
topics: ['sales-playbook', 'leads', 'opportunities'],
204+
sources: ['sales-playbook', 'leads', 'opportunities'],
205205
indexes: ['sales_docs'],
206206
},
207207
});
@@ -239,7 +239,7 @@ Always be empathetic and solution-focused.`,
239239
],
240240

241241
knowledge: {
242-
topics: ['support-kb', 'cases'],
242+
sources: ['support-kb', 'cases'],
243243
indexes: ['support_docs'],
244244
},
245245
});
@@ -313,7 +313,7 @@ Use the data tools to query records and aggregate metrics.`,
313313
],
314314

315315
knowledge: {
316-
topics: ['opportunities', 'pipeline'],
316+
sources: ['opportunities', 'pipeline'],
317317
indexes: ['sales_docs'],
318318
},
319319
});

content/docs/data-modeling/fields.mdx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,13 @@ only guard. Use `requiredWhen` for new metadata; keep `conditionalRequired`
347347
only for older packages that already emitted it. The alias is still accepted on
348348
input, but it is lowered into `requiredWhen` at parse time and then removed from
349349
the parsed metadata, so every consumer reads one canonical slot — if a field
350-
declares both, `requiredWhen` wins and the alias is discarded.
350+
declares both, `requiredWhen` wins and the alias is discarded. Declaring both
351+
with *different* predicates means the `conditionalRequired` one never gates the
352+
field, so it raises an advisory `field-requiredwhen-conditionalrequired-conflict`
353+
warning naming both — from `defineStack` when it parses your config, and from
354+
`os build` / `os validate` for stacks that skip strict `defineStack`. It never
355+
fails the build; the fix is to delete `conditionalRequired`. The same predicate
356+
in both slots is harmless duplication and stays quiet.
351357

352358
## Naming Conventions
353359

packages/spec/api-surface.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
".": [
33
"ACTION_TARGET_EXECUTE_CONFLICT (const)",
44
"ADMIN_FULL_ACCESS (const)",
5+
"AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT (const)",
56
"ALL_CONVERSIONS (const)",
67
"AUDIENCE_ANCHOR_POSITIONS (const)",
78
"Agent (type)",
@@ -51,6 +52,7 @@
5152
"ExpressionMetaSchema (const)",
5253
"ExpressionSchema (const)",
5354
"F (const)",
55+
"FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT (const)",
5456
"GUEST_POSITION (const)",
5557
"MAP_SUPPORTED_FIELDS (const)",
5658
"MEMBERSHIP_ROLE_ADMIN (const)",

packages/spec/src/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,13 @@ export type { MetadataCollectionInput, MapSupportedField, NormalizeStackInputOpt
110110
// Pre-parse authoring lint (#3743) — the one window where a deprecated alias is
111111
// still visible, since the parse itself resolves and drops it. `defineStack`
112112
// warns from here; the CLI runs the same rules over stacks that skip it.
113-
export { lintDeprecatedAliases, formatDeprecatedAliasFinding, ACTION_TARGET_EXECUTE_CONFLICT } from './shared/deprecated-aliases';
113+
export {
114+
lintDeprecatedAliases,
115+
formatDeprecatedAliasFinding,
116+
ACTION_TARGET_EXECUTE_CONFLICT,
117+
FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT,
118+
AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT,
119+
} from './shared/deprecated-aliases';
114120
export type { DeprecatedAliasFinding } from './shared/deprecated-aliases';
115121

116122
// Metadata conversion layer (ADR-0087 D2) — old-shape → canonical-shape transforms applied at load.

packages/spec/src/shared/deprecated-aliases.test.ts

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4-
import { lintDeprecatedAliases, ACTION_TARGET_EXECUTE_CONFLICT } from './deprecated-aliases';
4+
import {
5+
lintDeprecatedAliases,
6+
ACTION_TARGET_EXECUTE_CONFLICT,
7+
FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT,
8+
AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT,
9+
} from './deprecated-aliases';
510

611
// ── #3743: the discarded `execute` alias now has an author-facing warning ──
712
//
@@ -156,3 +161,156 @@ describe('lintDeprecatedAliases — action-target-execute-conflict', () => {
156161
]);
157162
});
158163
});
164+
165+
// ── The two sibling fold-and-drop aliases (#3743 follow-up) ──────────────────
166+
//
167+
// `execute` was never special: `FieldSchema` folds `conditionalRequired` into
168+
// `requiredWhen` and `AIKnowledgeSchema` folds `topics` into `sources`, both
169+
// dropping the alias from the parsed output exactly the same way. That makes
170+
// all three invisible to every post-parse check, so all three belong to this
171+
// pre-parse pass — which is the reason the pass exists as a rule SET rather
172+
// than one hard-coded rule.
173+
174+
describe('lintDeprecatedAliases — field-requiredwhen-conditionalrequired-conflict', () => {
175+
const objectWith = (field: Record<string, unknown>) => ({
176+
objects: [{
177+
name: 'crm_task',
178+
fields: { due_date: { type: 'date', label: 'Due', ...field } },
179+
}],
180+
});
181+
182+
it('flags a field declaring both predicates with different sources', () => {
183+
const findings = lintDeprecatedAliases(objectWith({
184+
requiredWhen: 'record.stage == "closed"',
185+
conditionalRequired: 'record.amount > 0',
186+
}));
187+
188+
expect(findings).toHaveLength(1);
189+
const [f] = findings;
190+
expect(f.rule).toBe(FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT);
191+
expect(f.severity).toBe('warning');
192+
expect(f.where).toBe(`field 'due_date' on object 'crm_task'`);
193+
expect(f.message).toContain('record.stage == "closed"');
194+
expect(f.message).toContain('record.amount > 0');
195+
expect(f.message).toContain(`'requiredWhen' wins`);
196+
expect(f.message).toContain('never gates the field');
197+
expect(f.hint).toContain(`Delete 'conditionalRequired'`);
198+
});
199+
200+
it('stays quiet when only one predicate slot is declared', () => {
201+
expect(lintDeprecatedAliases(objectWith({ requiredWhen: 'record.paid' }))).toEqual([]);
202+
expect(lintDeprecatedAliases(objectWith({ conditionalRequired: 'record.paid' }))).toEqual([]);
203+
});
204+
205+
it('compares the predicate TEXT, not the wrapper', () => {
206+
// A bare string and the `{ dialect, source }` envelope it lowers into are
207+
// the same predicate written two ways — nothing is lost either way.
208+
expect(lintDeprecatedAliases(objectWith({
209+
requiredWhen: 'record.paid',
210+
conditionalRequired: { dialect: 'cel', source: 'record.paid' },
211+
}))).toEqual([]);
212+
});
213+
214+
it('reads the envelope form on both slots', () => {
215+
const [f] = lintDeprecatedAliases(objectWith({
216+
requiredWhen: { dialect: 'cel', source: 'record.a' },
217+
conditionalRequired: { dialect: 'cel', source: 'record.b' },
218+
}));
219+
expect(f.rule).toBe(FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT);
220+
expect(f.message).toContain('record.a');
221+
expect(f.message).toContain('record.b');
222+
});
223+
224+
it('covers fields added by an object extension', () => {
225+
// An extension adds/overrides fields on another package's object through
226+
// the same `FieldSchema`, so it carries the same silent discard.
227+
const [f] = lintDeprecatedAliases({
228+
objectExtensions: [{
229+
objectName: 'sys_user',
230+
fields: { nickname: { type: 'text', requiredWhen: 'record.a', conditionalRequired: 'record.b' } },
231+
}],
232+
});
233+
expect(f.where).toBe(`field 'nickname' on object extension 'sys_user'`);
234+
});
235+
236+
it('tolerates an object with no fields', () => {
237+
expect(lintDeprecatedAliases({ objects: [{ name: 'crm_task' }] })).toEqual([]);
238+
});
239+
});
240+
241+
describe('lintDeprecatedAliases — agent-knowledge-sources-topics-conflict', () => {
242+
const agentWith = (knowledge: Record<string, unknown>) => ({
243+
agents: [{ name: 'support_bot', label: 'Support', knowledge }],
244+
});
245+
246+
it('flags an agent declaring both source lists with different members', () => {
247+
const findings = lintDeprecatedAliases(agentWith({
248+
sources: ['faq', 'policies'],
249+
topics: ['legacy_kb'],
250+
}));
251+
252+
expect(findings).toHaveLength(1);
253+
const [f] = findings;
254+
expect(f.rule).toBe(AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT);
255+
expect(f.severity).toBe('warning');
256+
expect(f.where).toBe(`agent 'support_bot' knowledge`);
257+
expect(f.message).toContain(`'faq', 'policies'`);
258+
expect(f.message).toContain(`'legacy_kb'`);
259+
expect(f.message).toContain(`'sources' wins`);
260+
expect(f.message).toContain('never recruited into RAG context');
261+
expect(f.hint).toContain(`Delete 'topics'`);
262+
});
263+
264+
it('stays quiet when only one list is declared', () => {
265+
expect(lintDeprecatedAliases(agentWith({ sources: ['faq'] }))).toEqual([]);
266+
expect(lintDeprecatedAliases(agentWith({ topics: ['faq'] }))).toEqual([]);
267+
});
268+
269+
it('stays quiet when the two lists have the same members in any order', () => {
270+
// Order and repetition are not meaningful for either slot, so the same set
271+
// written twice loses nothing.
272+
expect(lintDeprecatedAliases(agentWith({
273+
sources: ['faq', 'policies'],
274+
topics: ['policies', 'faq', 'faq'],
275+
}))).toEqual([]);
276+
});
277+
278+
it('treats an empty list as undeclared', () => {
279+
expect(lintDeprecatedAliases(agentWith({ sources: [], topics: ['faq'] }))).toEqual([]);
280+
});
281+
282+
it('tolerates an agent with no knowledge block', () => {
283+
expect(lintDeprecatedAliases({ agents: [{ name: 'support_bot' }] })).toEqual([]);
284+
});
285+
});
286+
287+
describe('lintDeprecatedAliases — all three rules together', () => {
288+
it('reports each rule independently in one pass', () => {
289+
const stack = {
290+
objects: [{
291+
name: 'crm_task',
292+
fields: { due_date: { type: 'date', requiredWhen: 'record.a', conditionalRequired: 'record.b' } },
293+
actions: [{ name: 'convert', type: 'script', target: 'preferred', execute: 'legacy' }],
294+
}],
295+
agents: [{ name: 'support_bot', knowledge: { sources: ['faq'], topics: ['legacy_kb'] } }],
296+
};
297+
298+
expect(lintDeprecatedAliases(stack).map((f) => f.rule).sort()).toEqual([
299+
ACTION_TARGET_EXECUTE_CONFLICT,
300+
AGENT_KNOWLEDGE_SOURCES_TOPICS_CONFLICT,
301+
FIELD_REQUIREDWHEN_CONDITIONALREQUIRED_CONFLICT,
302+
].sort());
303+
});
304+
305+
it('returns nothing for a stack that uses only canonical keys', () => {
306+
const stack = {
307+
objects: [{
308+
name: 'crm_task',
309+
fields: { due_date: { type: 'date', requiredWhen: 'record.a' } },
310+
actions: [{ name: 'convert', type: 'script', target: 'preferred' }],
311+
}],
312+
agents: [{ name: 'support_bot', knowledge: { sources: ['faq'] } }],
313+
};
314+
expect(lintDeprecatedAliases(stack)).toEqual([]);
315+
});
316+
});

0 commit comments

Comments
 (0)