Skip to content

Commit 6b695fd

Browse files
os-zhuangclaude
andcommitted
fix(autonumber): guard {field} interpolation footguns
Add three guardrails on top of the {field}/date/per-scope autonumber work: - Empty interpolated {field} now throws (shared missingFieldValues helper) in both the SQL driver and the engine fallback, instead of silently collapsing the record into the wrong counter scope. - Build-time lint (objectstack compile): unknown / self-referencing {field} fails the build; an optional {field} warns to mark it required. - Legacy _objectstack_sequences PK-widen failure fails safe — fixed-prefix sequences keep working and a per-scope write raises an actionable error rather than an opaque DB primary-key violation at insert time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0f8786c commit 6b695fd

11 files changed

Lines changed: 388 additions & 6 deletions

File tree

.changeset/autonumber-format-tokens.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"@objectstack/spec": minor
33
"@objectstack/objectql": minor
44
"@objectstack/driver-sql": minor
5+
"@objectstack/cli": minor
56
---
67

78
feat(autonumber): date, {field} and per-scope counter reset for autonumber formats
@@ -28,3 +29,19 @@ global counter, so existing sequences are unchanged. The persistent
2829
`_objectstack_sequences` table gains a `scope` column (PK widened to
2930
`object, tenant_id, field, scope`); deployments with the legacy 3-column table are
3031
migrated in place on first use, carrying existing counters to `scope=''`.
32+
33+
Guardrails against the `{field}` footguns:
34+
35+
- **Empty interpolated field is a hard error, not a silent mis-number.** A
36+
`{field}` token whose value is missing at create time would render to an empty
37+
prefix and collapse the record into the wrong counter scope. Both the SQL driver
38+
and the engine fallback now refuse to generate and throw a clear error naming the
39+
empty field (shared `missingFieldValues` helper).
40+
- **Build-time lint (`@objectstack/cli compile`).** `autonumber` formats are
41+
checked against the object's fields: a `{field}` token naming a non-existent
42+
field (or the autonumber field itself) **fails the build**; a token naming an
43+
*optional* field emits an advisory warning to mark it `required: true`.
44+
- **Legacy sequence-table migration fails safe.** If the legacy table's primary
45+
key cannot be widened to include `scope`, fixed-prefix sequences keep working and
46+
a per-scope write raises an actionable error instead of an opaque DB primary-key
47+
violation at insert time.

packages/cli/src/commands/compile.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { lowerCallables } from '../utils/lower-callables.js';
1111
import { validateStackExpressions } from '../utils/validate-expressions.js';
1212
import { validateWidgetBindings } from '../utils/validate-widget-bindings.js';
1313
import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
14+
import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
1415
import { lintLivenessProperties } from '../utils/lint-liveness-properties.js';
1516
import { collectAndLintDocs } from '../utils/collect-docs.js';
1617
import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js';
@@ -225,6 +226,38 @@ export default class Compile extends Command {
225226
}
226227
}
227228

229+
// 3d-ter. Autonumber `{field}` interpolation lint. A format like
230+
// `{plan_no}{000}` makes the referenced field part of the counter
231+
// scope, so it must exist and be set at create time — otherwise the
232+
// runtime throws (or, unlinted, silently mis-numbers). An unknown
233+
// field is broken → fails the build; an optional field is fragile →
234+
// advisory warning. Mirrors the broken/fragile two-level guardrail.
235+
const autonumberLint = lintAutonumberFormats(result.data as Record<string, unknown>);
236+
const autonumberErrors = autonumberLint.filter((f) => f.severity === 'error');
237+
const autonumberWarnings = autonumberLint.filter((f) => f.severity === 'warning');
238+
if (autonumberErrors.length > 0) {
239+
if (flags.json) {
240+
console.log(JSON.stringify({ success: false, error: 'autonumber format validation failed', issues: autonumberErrors }));
241+
this.exit(1);
242+
}
243+
console.log('');
244+
printError(`Autonumber format validation failed (${autonumberErrors.length} issue${autonumberErrors.length > 1 ? 's' : ''})`);
245+
for (const f of autonumberErrors) {
246+
console.log(` • ${f.where}: ${f.message}`);
247+
console.log(chalk.dim(` ${f.hint}`));
248+
console.log(chalk.dim(` rule: ${f.rule}`));
249+
}
250+
this.exit(1);
251+
}
252+
if (autonumberWarnings.length > 0 && !flags.json) {
253+
console.log('');
254+
for (const f of autonumberWarnings) {
255+
printWarning(`${f.where}: ${f.message}`);
256+
console.log(chalk.dim(` ${f.hint}`));
257+
console.log(chalk.dim(` rule: ${f.rule}`));
258+
}
259+
}
260+
228261
// 3d. Package docs (ADR-0046): compile flat `src/docs/*.md` into
229262
// `docs: DocSchema[]` and lint the combined set (flatness,
230263
// namespace-prefixed names, MDX/image ban, same-package link
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
lintAutonumberFormats,
6+
AUTONUMBER_UNKNOWN_FIELD,
7+
AUTONUMBER_OPTIONAL_FIELD,
8+
AUTONUMBER_SELF_REFERENCE,
9+
} from './lint-autonumber-formats.js';
10+
11+
describe('lintAutonumberFormats', () => {
12+
it('passes a date-only / fixed-prefix format with no {field} tokens', () => {
13+
const stack = {
14+
objects: [
15+
{ name: 'audit', fields: { audit_no: { type: 'autonumber', autonumberFormat: 'AD{YYYYMMDD}{0000}' } } },
16+
{ name: 'case', fields: { case_no: { type: 'autonumber', autonumberFormat: 'CASE-{0000}' } } },
17+
],
18+
};
19+
expect(lintAutonumberFormats(stack)).toEqual([]);
20+
});
21+
22+
it('passes when every {field} token is a required field on the object', () => {
23+
const stack = {
24+
objects: [
25+
{
26+
name: 'task',
27+
fields: {
28+
section: { type: 'text', required: true },
29+
island_zone: { type: 'text', required: true },
30+
task_no: { type: 'autonumber', autonumberFormat: '{section}{island_zone}{000}' },
31+
},
32+
},
33+
],
34+
};
35+
expect(lintAutonumberFormats(stack)).toEqual([]);
36+
});
37+
38+
it('errors when a {field} token names a non-existent field', () => {
39+
const stack = {
40+
objects: [
41+
{ name: 'task', fields: { task_no: { type: 'autonumber', autonumberFormat: '{plan_no}{000}' } } },
42+
],
43+
};
44+
const out = lintAutonumberFormats(stack);
45+
expect(out).toHaveLength(1);
46+
expect(out[0].severity).toBe('error');
47+
expect(out[0].rule).toBe(AUTONUMBER_UNKNOWN_FIELD);
48+
});
49+
50+
it('warns when a {field} token names an optional field', () => {
51+
const stack = {
52+
objects: [
53+
{
54+
name: 'task',
55+
fields: {
56+
plan_no: { type: 'text' }, // not required
57+
task_no: { type: 'autonumber', autonumberFormat: '{plan_no}{000}' },
58+
},
59+
},
60+
],
61+
};
62+
const out = lintAutonumberFormats(stack);
63+
expect(out).toHaveLength(1);
64+
expect(out[0].severity).toBe('warning');
65+
expect(out[0].rule).toBe(AUTONUMBER_OPTIONAL_FIELD);
66+
});
67+
68+
it('errors when the format interpolates the autonumber field itself', () => {
69+
const stack = {
70+
objects: [
71+
{ name: 'task', fields: { task_no: { type: 'autonumber', autonumberFormat: '{task_no}{000}' } } },
72+
],
73+
};
74+
const out = lintAutonumberFormats(stack);
75+
expect(out).toHaveLength(1);
76+
expect(out[0].severity).toBe('error');
77+
expect(out[0].rule).toBe(AUTONUMBER_SELF_REFERENCE);
78+
});
79+
80+
it('handles array-shaped fields and the `format` shorthand', () => {
81+
const stack = {
82+
objects: [
83+
{
84+
name: 'task',
85+
fields: [
86+
{ name: 'plan_no', type: 'text', required: true },
87+
{ name: 'task_no', type: 'autonumber', format: '{plan_no}{000}' },
88+
],
89+
},
90+
],
91+
};
92+
expect(lintAutonumberFormats(stack)).toEqual([]);
93+
});
94+
});
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Build-time lint for `autonumber` field formats. An `autonumberFormat` may
5+
* interpolate other fields of the same record (`{plan_no}{000}`,
6+
* `{section}{island_zone}{000}`). That field value forms the counter SCOPE, so
7+
* if it is missing at create time the record number silently collapses into the
8+
* wrong scope — and the runtime now throws rather than emit a wrong number
9+
* (see sql-driver / engine `missingFieldValues`). This lint catches the two
10+
* ways an author (very often an AI generating templates) gets that wrong,
11+
* BEFORE it ships:
12+
*
13+
* - ERROR: `{field}` names a field that does not exist on the object — the
14+
* generation will always throw. This is broken, so it fails the build.
15+
* - WARNING: `{field}` names an OPTIONAL field — generation throws on any
16+
* record left blank. The robust shape marks the referenced field
17+
* `required: true` (mirroring ERPNext/Odoo, where a field that drives the
18+
* naming series must be mandatory). Advisory; does not fail the build.
19+
*
20+
* A self-reference (`{self}` on the autonumber field itself) is always an
21+
* ERROR — the value does not exist yet when the format renders.
22+
*/
23+
24+
import { parseAutonumberFormat, referencedFields } from '@objectstack/spec/data';
25+
26+
export interface AutonumberLintFinding {
27+
where: string;
28+
message: string;
29+
hint: string;
30+
rule: string;
31+
severity: 'error' | 'warning';
32+
}
33+
34+
type AnyRec = Record<string, unknown>;
35+
36+
export const AUTONUMBER_UNKNOWN_FIELD = 'autonumber-references-unknown-field';
37+
export const AUTONUMBER_OPTIONAL_FIELD = 'autonumber-references-optional-field';
38+
export const AUTONUMBER_SELF_REFERENCE = 'autonumber-references-self';
39+
40+
function asArray(v: unknown): AnyRec[] {
41+
if (Array.isArray(v)) return v as AnyRec[];
42+
if (v && typeof v === 'object') {
43+
return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) }));
44+
}
45+
return [];
46+
}
47+
48+
/**
49+
* Lint every `autonumber` field's format for unresolvable / fragile `{field}`
50+
* interpolation. Returns a (possibly empty) list of findings; never throws.
51+
*/
52+
export function lintAutonumberFormats(stack: AnyRec): AutonumberLintFinding[] {
53+
const findings: AutonumberLintFinding[] = [];
54+
for (const obj of asArray(stack.objects)) {
55+
const objectName = typeof obj.name === 'string' ? obj.name : '(unnamed object)';
56+
const fields = asArray(obj.fields);
57+
// name → required?, for schema-aware reference checks.
58+
const fieldMeta = new Map<string, { required: boolean }>();
59+
for (const f of fields) {
60+
if (typeof f.name === 'string') fieldMeta.set(f.name, { required: f.required === true });
61+
}
62+
63+
for (const f of fields) {
64+
if (f.type !== 'autonumber') continue;
65+
const name = typeof f.name === 'string' ? f.name : '(unnamed field)';
66+
const fmt = typeof f.autonumberFormat === 'string'
67+
? f.autonumberFormat
68+
: (typeof f.format === 'string' ? f.format : '');
69+
if (!fmt) continue;
70+
const refs = referencedFields(parseAutonumberFormat(fmt));
71+
const where = `object '${objectName}' · field '${name}' (autonumber "${fmt}")`;
72+
for (const ref of refs) {
73+
if (ref === name) {
74+
findings.push({
75+
where,
76+
message: `format interpolates \`{${ref}}\` — its own value, which does not exist yet when the number is generated.`,
77+
hint: `Reference a DIFFERENT field that is set before create (e.g. \`{plan_no}{000}\`), or drop the token.`,
78+
rule: AUTONUMBER_SELF_REFERENCE,
79+
severity: 'error',
80+
});
81+
continue;
82+
}
83+
const meta = fieldMeta.get(ref);
84+
if (!meta) {
85+
findings.push({
86+
where,
87+
message: `format interpolates \`{${ref}}\`, but object '${objectName}' has no field named '${ref}' — generation will always throw.`,
88+
hint: `Reference an existing field, or remove the \`{${ref}}\` token from the format.`,
89+
rule: AUTONUMBER_UNKNOWN_FIELD,
90+
severity: 'error',
91+
});
92+
} else if (!meta.required) {
93+
findings.push({
94+
where,
95+
message: `format interpolates \`{${ref}}\`, but '${ref}' is optional — any record left blank fails autonumber generation at create time.`,
96+
hint: `Mark '${ref}' as \`required: true\` so it is always set before the record number is rendered.`,
97+
rule: AUTONUMBER_OPTIONAL_FIELD,
98+
severity: 'warning',
99+
});
100+
}
101+
}
102+
}
103+
}
104+
return findings;
105+
}

packages/objectql/src/engine-autonumber-defer.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,22 @@ describe('ObjectQL autonumber ownership (#1603)', () => {
151151
// Today's UTC day + a fresh per-day counter.
152152
expect(r.audit_no).toMatch(/^AD\d{8}0001$/);
153153
});
154+
155+
it('fallback refuses to generate when an interpolated {field} is empty', async () => {
156+
const TASK_SCHEMA = {
157+
name: 'task',
158+
fields: {
159+
zone: { type: 'text' },
160+
task_no: { type: 'autonumber', format: '{zone}{000}' },
161+
},
162+
};
163+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(TASK_SCHEMA as any);
164+
const driver = makeDriver(false);
165+
engine.registerDriver(driver, true);
166+
await engine.init();
167+
168+
// zone left blank → the prefix collapses to '' and would mis-scope the
169+
// counter, so generation must throw rather than emit a wrong number.
170+
await expect(engine.insert('task', {})).rejects.toThrow(/zone/);
171+
});
154172
});

packages/objectql/src/engine.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
EngineAggregateOptions,
1111
EngineCountOptions
1212
} from '@objectstack/spec/data';
13-
import { parseAutonumberFormat, renderAutonumber } from '@objectstack/spec/data';
13+
import { parseAutonumberFormat, renderAutonumber, missingFieldValues } from '@objectstack/spec/data';
1414
import { ExecutionContext, ExecutionContextSchema } from '@objectstack/spec/kernel';
1515
import { DriverInterface, IDataEngine, Logger, createLogger } from '@objectstack/core';
1616
import { CoreServiceName, StorageNameMapping } from '@objectstack/spec/system';
@@ -826,6 +826,17 @@ export class ObjectQL implements IDataEngine {
826826
// `format` (both appear in metadata; the driver reads both too) — #1603.
827827
const fmt = (def as any).autonumberFormat ?? (def as any).format;
828828
const tokens = parseAutonumberFormat(typeof fmt === 'string' ? fmt : '');
829+
// Refuse to generate when an interpolated `{field}` is empty — it would
830+
// render to an empty prefix and merge this record into the wrong counter
831+
// scope. Mirror the SQL driver so both paths fail identically (#1603).
832+
const missing = missingFieldValues(tokens, record);
833+
if (missing.length > 0) {
834+
throw new Error(
835+
`Cannot generate autonumber "${object}.${name}" (format "${fmt}"): ` +
836+
`referenced field(s) [${missing.join(', ')}] are empty on the record. ` +
837+
`Fields interpolated into an autonumber format must be set before the record is created.`,
838+
);
839+
}
829840
// The counter scope is the rendered prefix (date/field tokens before the
830841
// sequence slot); it is independent of the counter value, so a throwaway
831842
// render with seq 0 yields the scope and the literal prefix to seed from.

packages/plugins/driver-sql/src/sql-driver-autonumber-tokens.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,21 @@ describe('SqlDriver auto_number format tokens', () => {
123123
expect(b.plan_no).toBe(`PROD${utcYmd()}001`);
124124
});
125125

126+
it('refuses to generate when an interpolated {field} is empty (no silent mis-scope)', async () => {
127+
await driver.initObjects([
128+
{
129+
name: 'dispatch_order',
130+
fields: {
131+
plan_no: { type: 'string' },
132+
order_no: { type: 'autonumber', format: '{plan_no}{000}' },
133+
},
134+
},
135+
]);
136+
// plan_no left blank → the prefix would collapse to '' and merge this row
137+
// into the wrong counter scope, so generation must throw instead.
138+
await expect(driver.create('dispatch_order', {})).rejects.toThrow(/plan_no/);
139+
});
140+
126141
it('leaves fixed-prefix formats on a single global counter (backward compatible)', async () => {
127142
await driver.initObjects([
128143
{ name: 'invoice', fields: { invoice_number: { type: 'autonumber', format: 'INV-{0000}' } } },

0 commit comments

Comments
 (0)