Skip to content

Commit 73ad0bb

Browse files
os-trumpclaude
andauthored
refactor(runtime): spell the object-less action key as the constant (#14678) (#14861)
`GLOBAL_ACTION_OBJECT_KEY` exists so the object-less action-registration key is written once. #14422 converged the owner-key ladder and the ObjectQL plugin's copy of it; three bare `'global'` spellings elsewhere in `packages/runtime/src/action-execution.ts` were never in that card's path, because the runtime fence it built was a re-export plus a delegating alias. All three are equal in value to the constant today — which is the defect, not a mitigation: it is what made them invisible to every test in the repo, and what would have let them part from the constant in silence. - `seedFlowActionParams`: the live comparison deciding whether an object-derived `<object>Id` param key is seeded. Substitution only — the predicate is unchanged, so `'*'` still falls through exactly as before. - `enforceActionParams`: the warn-once dedup key, also interpolated into the operator-facing `[action-params] <key>: …` line. - `collectActionDeclarations`'s docblock, which also called the key "the wildcard" against `action-governance.ts`'s "no wildcard semantics". It now reads as the sibling docblock 48 lines below already did. `action-owner-key-single-source.test.ts` gains a half C that reads `action-execution.ts` and fails if any quote spelling is written out by hand again. The forbidden spelling is derived from the constant, so the guard is not itself another copy of the literal it forbids. Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza Co-authored-by: Claude <noreply@anthropic.com>
1 parent 431979e commit 73ad0bb

3 files changed

Lines changed: 102 additions & 3 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
refactor(runtime): spell the object-less action key as `GLOBAL_ACTION_OBJECT_KEY` in `action-execution.ts` (#14678)
6+
7+
`GLOBAL_ACTION_OBJECT_KEY` exists so the object-less action-registration key is
8+
written once. #14422 converged the owner-key LADDER and the ObjectQL plugin's
9+
copy of it; three bare `'global'` spellings elsewhere in
10+
`packages/runtime/src/action-execution.ts` were never in that card's path,
11+
because the runtime fence it built was a re-export plus a delegating alias.
12+
This converges those three. The constant was already imported in the file.
13+
14+
No behaviour moves — the constant is `'global'`, so every site is equal in
15+
value before and after. That equality is the entire defect: it is what made the
16+
three invisible to every test in the repo, and what would have let them part
17+
from the constant in silence the day its value changes.
18+
19+
- `seedFlowActionParams` — a live comparison (`objectName !== 'global'`) that
20+
decides whether an object-derived `<object>Id` param key is seeded. The one
21+
site where a drifted literal would change what an action body receives.
22+
- `enforceActionParams` — the warn-once dedup key, which is also interpolated
23+
into the operator-facing `[action-params] <key>: …` line. Converged rather
24+
than left: the argument for a literal here is that a log key must never fail
25+
to render, and that argument does not survive contact with the fact that
26+
`GLOBAL_ACTION_OBJECT_KEY` is a module-scope `const string` already imported
27+
into this file — it cannot fail to render either. What a drift there would
28+
actually cost is an operator grepping logs by the key the engine now uses and
29+
silently missing these lines.
30+
- `collectActionDeclarations`'s docblock, which carried a second defect
31+
independent of the literal: it called the key "the `'global'` wildcard",
32+
contradicting `action-governance.ts` ("an exact-string `Map` lookup with no
33+
wildcard semantics"). It is now the phrasing the sibling docblock 48 lines
34+
below it already used — "the object-less `GLOBAL_ACTION_OBJECT_KEY`" — so the
35+
correction is copied from the file's own converged prose rather than invented.
36+
37+
`patch`, not `skip-changeset`: `packages/runtime` publishes `dist`, which is
38+
built from this source, so the emitted bytes move even though the behaviour
39+
does not. Nothing reaches the published entry — `action-execution.ts` is not
40+
re-exported from `packages/runtime/src/index.ts` and no export, signature or
41+
type changed here — which is what keeps it below `minor`.
42+
43+
The docblock that promised the lockstep is joined by a weld that enforces it:
44+
`action-owner-key-single-source.test.ts` gains a half C that reads
45+
`action-execution.ts` and fails if any quote spelling of the key is written out
46+
by hand again. The forbidden spelling is DERIVED from the constant rather than
47+
hard-coded, so the guard is not itself a fourth copy of the literal it forbids.

packages/runtime/src/action-execution.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ export function seedFlowActionParams(_deps: ActionExecutionDeps,
638638

639639
if (rowId != null) {
640640
const keys = new Set<string>(['recordId']);
641-
if (objectName && objectName !== 'global') {
641+
if (objectName && objectName !== GLOBAL_ACTION_OBJECT_KEY) {
642642
keys.add(`${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`);
643643
}
644644
if (typeof action?.recordIdParam === 'string' && action.recordIdParam) {
@@ -1013,7 +1013,7 @@ export function enforceActionParams(deps: ActionExecutionDeps,
10131013
if (!laxActionParams()) {
10141014
return `Invalid action params: ${summary}`;
10151015
}
1016-
const key = `${where.objectName ?? 'global'}/${where.actionName ?? action?.name ?? 'action'}`;
1016+
const key = `${where.objectName ?? GLOBAL_ACTION_OBJECT_KEY}/${where.actionName ?? action?.name ?? 'action'}`;
10171017
warnActionParamsOnce(
10181018
key,
10191019
`[action-params] ${key}: ${summary} — accepted because ` +
@@ -1487,7 +1487,7 @@ export async function resolveActionByName(deps: ActionExecutionDeps,
14871487
* engine executes since #2608 (`resyncAuthoredActions`) but that never
14881488
* appear inside any object definition. Their owning object follows the
14891489
* same convention as the engine registration key (`objectName` field,
1490-
* legacy `object` field, else the `'global'` wildcard).
1490+
* legacy `object` field, else the object-less `GLOBAL_ACTION_OBJECT_KEY`).
14911491
*
14921492
* On a key clash (`objectName:name`) the object-embedded declaration wins,
14931493
* mirroring the execution layer's artifact-wins rule — `resyncAuthoredActions`

packages/runtime/src/action-owner-key-single-source.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@
2525
* copy: a byte-identical second spelling passes every assertion in half A. Half
2626
* B reads this package's own source and fails if the ladder grows a second
2727
* body here.
28+
*
29+
* Half C closes the same hole one level down (#14678). #14422 converged the
30+
* LADDER, and the runtime kept three bare `'global'` spellings elsewhere in
31+
* `action-execution.ts` that the ladder check could not see: a live comparison
32+
* in `seedFlowActionParams`, a warn-once log key in `enforceActionParams`, and
33+
* a docblock. All three were equal in value and invisible to every test in the
34+
* repo, which is the whole shape #14422 was filed to remove — so the same
35+
* convergence needed the same weld, or the next reader re-inlines one and
36+
* nothing says so.
2837
*/
2938

3039
import { readFileSync } from 'node:fs';
@@ -89,6 +98,25 @@ function readActionExecutionSource(): string {
8998
return readFileSync(join(here, 'action-execution.ts'), 'utf8');
9099
}
91100

101+
/**
102+
* Quote spellings of the object-less key as a bare literal, DERIVED from the
103+
* constant rather than hard-coded.
104+
*
105+
* Deriving it is the point, not a flourish. A hard-coded `'global'` here would
106+
* be a fourth copy of the very literal this file exists to forbid, and it
107+
* would go stale in the same silence the day the constant moves. Derived, the
108+
* guard follows the constant: whatever `GLOBAL_ACTION_OBJECT_KEY` becomes,
109+
* that is the spelling `action-execution.ts` may not write out by hand. The
110+
* re-inlining it catches is caught at the moment it happens, while the two
111+
* spellings are still equal — which is the only moment a reader can tell they
112+
* were ever meant to be one thing.
113+
*/
114+
const BARE_LITERALS: readonly string[] = [
115+
`'${GLOBAL_ACTION_OBJECT_KEY}'`,
116+
`"${GLOBAL_ACTION_OBJECT_KEY}"`,
117+
`\`${GLOBAL_ACTION_OBJECT_KEY}\``,
118+
];
119+
92120
describe('standalone-action owner key — half B: one spelling (#14422)', () => {
93121
it('keeps no ladder body of its own in action-execution.ts', () => {
94122
const src = readActionExecutionSource();
@@ -112,3 +140,27 @@ describe('standalone-action owner key — half B: one spelling (#14422)', () =>
112140
expect(body[1].trim()).toBe('return standaloneActionOwnerKey(action);');
113141
});
114142
});
143+
144+
describe('standalone-action owner key — half C: no bare literal (#14678)', () => {
145+
it('spells the object-less key as the CONSTANT everywhere in action-execution.ts', () => {
146+
const src = readActionExecutionSource();
147+
148+
// Anti-vacuity, twice over. An empty read, or a file that does not
149+
// import the constant at all, would make every negative below pass for
150+
// exactly the wrong reason — the can-never-fail property this whole
151+
// file was written to replace. Both controls are positive assertions
152+
// against text the converged file must carry.
153+
expect(src).toContain('GLOBAL_ACTION_OBJECT_KEY');
154+
expect(src).toContain('objectName !== GLOBAL_ACTION_OBJECT_KEY');
155+
156+
for (const literal of BARE_LITERALS) {
157+
expect(
158+
src.includes(literal),
159+
`action-execution.ts writes the object-less action key as the bare literal `
160+
+ `${literal}. It is equal in value to GLOBAL_ACTION_OBJECT_KEY today and parts `
161+
+ `from it in silence the day the constant moves (#14422, #14678). Import the `
162+
+ `constant — this file already does — and compare or interpolate that instead.`,
163+
).toBe(false);
164+
}
165+
});
166+
});

0 commit comments

Comments
 (0)