Skip to content

Commit f6a04e3

Browse files
fix(runtime): a flow clone the engine refuses no longer installs the source's empty customization package (#29193)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9NoBsyXUn3Gyjgj4BbxoetR)_ Fixes #29016 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:流程克隆接口(不带 package 时)现在先让引擎校验副本,校验通过后才创建源包的 `.custom` 配对包。 - 为什么改:原来先建配对包、后校验,名字非法被 400 拒绝后,包列表里仍多出一个空的 `.custom` 包;被拒绝的写入应该什么都不改变。 - 风险与回滚:只调整克隆接口内部顺序,成功路径的结果不变;回滚即还原本 PR 一个文件的改动。 - 先看:`packages/runtime/src/domains/automation.ts` 的 `resolveCloneTarget`(拆成"先判定、后 land")与 `registerAndSaveFlow` 新增的 `land` 段(失败时撤销注册)。 - 两分钟验证:`pnpm --filter @objectstack/runtime exec vitest run src/domains/automation-flow-clone-custom-package.test.ts`,预期 17 个用例全绿,其中 `[#29016]` 两例。 - 需要你做的:无,席位落地。 ## What changes `POST /api/v1/automation/:name/clone` with `package` omitted used to ensure the source's paired customization package `BASE.custom` in `resolveCloneTarget`, before the engine's registration ran its strict `FlowSchema` parse. A clone refused `400 VALIDATION_FAILED` for an illegal machine name therefore left a newly installed, empty pair behind. - `resolveCloneTarget` now makes every judgement that needs no pair up front (the `501` for a protocol that cannot ensure, the omitted-`package` `400` for a source no package ships, the explicit target's rule) and returns a `land` step instead of a package id. For an explicit target, `land` returns the already-judged package; for the default target it ensures the pair and judges it. - `registerAndSaveFlow` takes that `land` step in place of the bare `packageId` and calls it **after** the engine has accepted the definition and **before** the save. A landing that refuses or throws undoes the registration through the existing `restoreFlowRegistration`, the same way a refused save does. - One validation spelling: the engine's registration stays the only check of the copy's definition; no second `FlowSchema` parse was added to the door. ### Refusals after the first possible install, measured | Refusal | Before | After | |:---|:---|:---| | Engine registration (strict parse: name pattern, cycles, node config keys, expressions) | pair stays | nothing installed (pinned) | | Authoring rule (`refuseUnauthoredFlowWrite`) | pair stays (it ran after the ensure) | nothing installed (now runs before `land`) | | Ensured pair judged not writable by the target rule | pair stays | pair stays (only knowable after the ensure) | | Store refuses the save (name already held in the pair, two-package `409`, store failure) | pair stays | pair stays (needs the store; no rollback machine added) | | Permission, walled write | judged before the domain is entered | unchanged | The last two rows are listed in the acceptance notes, not fixed here. ## Tests - New describe `[#29016] a clone the engine refuses installs no pair` in `automation-flow-clone-custom-package.test.ts`. The rig's engine gains an opt-in `parse` mode (a strict `FlowSchema.parse`, which is what `registerFlow` runs first), with a control case (legal name lands in the ensured pair) and the pin: `Bad-Name!` is refused `400 VALIDATION_FAILED` on `name`, the installed-package list is unchanged, nothing is persisted as a package, and no copy is registered or stored. - No existing test was edited. The explicit-target refusal cases still assert `registerFlow` was never called. - Ablation: the pair ensured eagerly again in `resolveCloneTarget` (through `scripts/ablation-replace.mjs`, marker landed 1, anchor 1 to 0), the pin goes red, restored to `git diff HEAD` empty: ``` × an illegal machine name is refused 400 and the package list is unchanged AssertionError: expected [ 'com.acme.crm', …(4) ] to deeply equal [ Array(4) ] + "com.acme.crm.custom", Tests 1 failed | 16 passed (17) ``` ## Acceptance notes - A clone whose save the store refuses after the pair was ensured (the new name already stored in the pair but not bound in the engine, or a store failure) still leaves a freshly ensured pair installed. Reach: needs a pre-existing stored row or a store fault; the pair is the one Studio's New would ensure and it is empty. Closing it needs either a store probe before the ensure or a pair rollback, both outside this card. - The default pair being judged not writable right after its own ensure is only reachable if the ensure door answers a pair this door cannot write into. ## Verification All on head `3de5e31ba` (after merging `origin/main` once): - `pnpm --filter '@objectstack/runtime^...' build` then `vitest run src/domains/automation` under `os-verify-lock`: Test Files 42 passed, Tests 766 passed, `VERDICT command-exit 0`. - `pnpm --filter @objectstack/runtime typecheck`: `VERDICT command-exit 0`. - `eslint --no-inline-config` on the two touched runtime files: exit 0. - `dispatch-gates --commands`: 61 derived, 60 run with exit 0 (the teardown-shape self-test after fetching its pinned fixture commit). `check:dual-build-cjs-loads` is NOT MEASURED locally (exit 3, PREREQUISITE NOT MET: needs a full workspace build); CI's run is the reading. --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmPX8EnvNFVzRVU5NHx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent eb14f7a commit f6a04e3

3 files changed

Lines changed: 131 additions & 37 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): a flow clone the engine refuses no longer installs the source's empty `.custom` customization package
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`POST /api/v1/automation/:name/clone` without `package` lands the copy in the source's paired customization package `<base>.custom`, ensuring it if needed. The pair used to be ensured before the engine validated the copy, so a clone refused `400` for an illegal machine name (or any other definition the engine refuses) still left a newly installed, empty `<base>.custom` in the package list and in Studio. The pair is now ensured only after the engine has accepted the copy, so a refused clone changes nothing. Refusals that can only come later — the ensured pair judged not writable, or the store refusing the save — still leave the pair in place, which is its ordinary state: ensure is idempotent, an empty pair is valid, and the next clone reuses it.

‎packages/runtime/src/domains/automation-flow-clone-custom-package.test.ts‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import { SqlDriver } from '@objectstack/driver-sql';
4242
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
4343
import { SysMetadata, SysMetadataHistoryObject, SysMetadataAuditObject } from '@objectstack/platform-objects/metadata';
4444
import { CloneFlowRequestSchema, CloneFlowResponseSchema } from '@objectstack/spec/api';
45+
import { FlowSchema } from '@objectstack/spec/automation';
4546
import { ROUTE_LEDGER } from '../route-ledger.js';
4647
import { HttpDispatcher } from '../http-dispatcher.js';
4748
import { validationFailureDetails, VALIDATION_FAILED_STATUS } from '../validation-failure.js';
@@ -97,7 +98,13 @@ interface Answer {
9798
body: any;
9899
}
99100

100-
async function boot() {
101+
/**
102+
* `parse`: the engine's registration as a strict `FlowSchema` parse — the
103+
* check `registerFlow` runs first (`canonicalizeFlow`), so a definition the
104+
* protocol refuses (an illegal machine name among it) throws the way the real
105+
* engine throws. The default rig's engine is a map that accepts anything.
106+
*/
107+
async function boot(options: { parse?: boolean } = {}) {
101108
const engine = new ObjectQL();
102109
liveEngines.push(engine);
103110
engine.registerDriver(new SqlDriver({
@@ -126,7 +133,11 @@ async function boot() {
126133
const protocol = new ObjectStackProtocolImplementation(engine as any, () => services);
127134

128135
const flows = new Map<string, any>([[SOURCE, shipped], [LOCAL, flowOf(LOCAL)]]);
129-
const registerFlow = vi.fn((name: string, definition: unknown) => { flows.set(name, definition); return definition; });
136+
const registerFlow = vi.fn((name: string, definition: unknown) => {
137+
if (options.parse) FlowSchema.parse(definition);
138+
flows.set(name, definition);
139+
return definition;
140+
});
130141
const unregisterFlow = vi.fn((name: string) => { flows.delete(name); });
131142
const getFlow = vi.fn(async (name: string) => flows.get(name) ?? null);
132143
const named: Record<string, unknown> = {
@@ -310,6 +321,37 @@ describe('[#28149] a source no package ships has no pair', () => {
310321
}, 60_000);
311322
});
312323

324+
describe('[#29016] a clone the engine refuses installs no pair', () => {
325+
it('control — with the parsing engine, a legal name still lands in the ensured pair', async () => {
326+
const rig = await boot({ parse: true });
327+
328+
const answer = await rig.clone(SOURCE, { name: 'acme_lead_followup', label: 'Lead follow-up (ACME)' });
329+
330+
expect(answer.status, JSON.stringify(answer.body)).toBe(200);
331+
expect(answer.data.packageId).toBe(PAIR);
332+
expect(rig.installed()).toContain(PAIR);
333+
}, 60_000);
334+
335+
it('an illegal machine name is refused 400 and the package list is unchanged', async () => {
336+
const rig = await boot({ parse: true });
337+
const before = rig.installed();
338+
const publishedBefore = rig.published.length;
339+
340+
const answer = await rig.clone(SOURCE, { name: 'Bad-Name!', label: 'Illegal' });
341+
342+
expect(answer.status, JSON.stringify(answer.body)).toBe(400);
343+
expect(answer.code).toBe('VALIDATION_FAILED');
344+
expect(answer.body.error.details.fields).toMatchObject([{ field: 'name' }]);
345+
// A refused clone changes nothing: no pair installed or persisted…
346+
expect(rig.installed()).toEqual(before);
347+
expect(rig.installed()).not.toContain(PAIR);
348+
expect(rig.published.length).toBe(publishedBefore);
349+
// …and no copy registered or stored.
350+
expect(rig.flows.has('Bad-Name!')).toBe(false);
351+
expect(await rig.rows('Bad-Name!')).toEqual([]);
352+
}, 60_000);
353+
});
354+
313355
describe('[#28149] the contract: the ledger row names the schemas this suite parses with', () => {
314356
it('the clone row is SDK surface and names CloneFlowRequestSchema / CloneFlowResponseSchema', () => {
315357
const row = ROUTE_LEDGER.find((e) => e.route === 'POST /automation/:name/clone');

‎packages/runtime/src/domains/automation.ts‎

Lines changed: 77 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,12 +2337,16 @@ async function registerAndSaveFlow(
23372337
definition: unknown,
23382338
door: 'definition' | 'clone',
23392339
/**
2340-
* [#28149] The package the clone door resolved for its copy
2340+
* [#28149, #29016] Where the clone door's copy lands
23412341
* ({@link resolveCloneTarget}) — the one caller here that holds a package
23422342
* of its own choosing rather than a stamp. The definition doors pass none.
2343+
* Asked AFTER the engine has accepted the definition and before the save:
2344+
* landing may install the source's paired customization package, and a
2345+
* definition the engine refuses must leave nothing installed. A landing
2346+
* that refuses (or throws) undoes the registration like a refused save.
23432347
*/
2344-
packageId?: string,
2345-
): Promise<{ registered: unknown } | { refused: HttpDispatcherResult }> {
2348+
land?: () => Promise<CloneLanding>,
2349+
): Promise<{ registered: unknown; packageId?: string } | { refused: HttpDispatcherResult }> {
23462350
const store: (Partial<Pick<ObjectStackProtocolImplementation, 'saveMetaItem'>>
23472351
& Partial<Pick<MetadataProtocol, 'getMetaItems'>>) | undefined =
23482352
await deps.resolveServiceOrLoud(context, 'protocol');
@@ -2355,6 +2359,21 @@ async function registerAndSaveFlow(
23552359
refused: { handled: true, response: deps.errorFromThrown(flowDefinitionRefusal(e), VALIDATION_FAILED_STATUS) },
23562360
};
23572361
}
2362+
let packageId: string | undefined;
2363+
if (land) {
2364+
let landing: CloneLanding;
2365+
try {
2366+
landing = await land();
2367+
} catch (e) {
2368+
restoreFlowRegistration(automationService, name, held);
2369+
throw e;
2370+
}
2371+
if ('refused' in landing) {
2372+
restoreFlowRegistration(automationService, name, held);
2373+
return landing;
2374+
}
2375+
packageId = landing.packageId;
2376+
}
23582377
if (typeof store?.saveMetaItem === 'function') {
23592378
// [#27414, ADR-0073 D3] The save names who asked: the caller's user id
23602379
// as the save's `actor`, which the store records in
@@ -2387,9 +2406,12 @@ async function registerAndSaveFlow(
23872406
};
23882407
}
23892408
}
2390-
return { registered };
2409+
return { registered, ...(packageId !== undefined ? { packageId } : {}) };
23912410
}
23922411

2412+
/** [#28149] The package a clone's copy lands in, or the refusal that stops it. */
2413+
type CloneLanding = { packageId: string } | { refused: HttpDispatcherResult };
2414+
23932415
/**
23942416
* [#28149, ADR-0146 D3/D4] WHERE the clone door's copy lands — "new similar":
23952417
* a package, always (ADR-0070 D5/D6 retired the package-less scope, and ⛔ no
@@ -2423,17 +2445,25 @@ async function registerAndSaveFlow(
24232445
* has nowhere to land a copy: `501`, never the package-less write it used to
24242446
* make.
24252447
*
2426-
* Ordering: the pair is ensured before the copy is registered and saved, so a
2427-
* save the store then refuses leaves the pair in place. That is the pair's
2428-
* ordinary state — ensure is idempotent and an empty pair is a valid one —
2429-
* and the next clone reuses it.
2448+
* Ordering [#29016] — a refused clone installs nothing it can avoid. Every
2449+
* judgement that needs no pair (the 501 above, the omitted-`package` 400 for a
2450+
* source no package ships, the explicit target's rule) is made HERE, before
2451+
* anything is registered. The pair itself is ensured only by the returned
2452+
* `land`, which {@link registerAndSaveFlow} calls after the engine has
2453+
* accepted the copy: the engine's registration is the one validation of a
2454+
* flow definition (strict parse — the name pattern among it — cycles, node
2455+
* config keys, expressions), so a copy it refuses is refused before any pair
2456+
* exists, with no second copy of that check here. A refusal that can only
2457+
* come later — the ensured pair judged not writable, or the save refused —
2458+
* leaves the pair in place: that is the pair's ordinary state (ensure is
2459+
* idempotent and an empty pair is a valid one) and the next clone reuses it.
24302460
*/
24312461
async function resolveCloneTarget(
24322462
deps: DomainHandlerDeps,
24332463
context: HttpProtocolContext,
24342464
sourceName: string,
24352465
requested: string | undefined,
2436-
): Promise<{ packageId: string } | { refused: HttpDispatcherResult }> {
2466+
): Promise<{ land: () => Promise<CloneLanding> } | { refused: HttpDispatcherResult }> {
24372467
const protocol: Partial<Pick<ObjectStackProtocolImplementation,
24382468
'ensureCustomizationPackage' | 'packagedArtifactOwner' | 'saveMetaItem'>> | undefined =
24392469
await deps.resolveServiceOrLoud(context, 'protocol');
@@ -2453,30 +2483,38 @@ async function resolveCloneTarget(
24532483
}
24542484
const subject = `flow '${sourceName}'`;
24552485
const owner = protocol.packagedArtifactOwner({ type: FLOW_METADATA_TYPE, name: sourceName });
2456-
let target: string;
2486+
const ensure = protocol.ensureCustomizationPackage;
2487+
const judge = async (target: string): Promise<CloneLanding> => {
2488+
const refusal = newSimilarTargetRefusal(await deps.getObjectQL(context), {
2489+
target,
2490+
subject,
2491+
...(owner !== undefined ? { sourcePackageId: owner } : {}),
2492+
});
2493+
return refusal ? { refused: newSimilarTargetRefusalAnswer(deps, refusal) } : { packageId: target };
2494+
};
24572495
if (requested !== undefined) {
2458-
target = requested;
2459-
} else {
2460-
if (owner === undefined) {
2461-
throw validationFailure(
2462-
`Flow '${sourceName}' is not shipped by a package, so it has no paired customization package to `
2463-
+ 'land a copy in (ADR-0146 D3): name the package the copy lands in with `package`.',
2464-
[{ field: 'package', code: 'required', message: 'the source has no paired customization package' }],
2465-
);
2466-
}
2467-
try {
2468-
target = (await protocol.ensureCustomizationPackage({ basePackageId: owner })).packageId;
2469-
} catch (e) {
2470-
return { refused: { handled: true, response: deps.errorFromThrown(e) } };
2471-
}
2496+
const landing = await judge(requested);
2497+
if ('refused' in landing) return landing;
2498+
return { land: async () => landing };
24722499
}
2473-
const refusal = newSimilarTargetRefusal(await deps.getObjectQL(context), {
2474-
target,
2475-
subject,
2476-
...(owner !== undefined ? { sourcePackageId: owner } : {}),
2477-
});
2478-
if (refusal) return { refused: newSimilarTargetRefusalAnswer(deps, refusal) };
2479-
return { packageId: target };
2500+
if (owner === undefined) {
2501+
throw validationFailure(
2502+
`Flow '${sourceName}' is not shipped by a package, so it has no paired customization package to `
2503+
+ 'land a copy in (ADR-0146 D3): name the package the copy lands in with `package`.',
2504+
[{ field: 'package', code: 'required', message: 'the source has no paired customization package' }],
2505+
);
2506+
}
2507+
return {
2508+
land: async () => {
2509+
let pair: string;
2510+
try {
2511+
pair = (await ensure.call(protocol, { basePackageId: owner })).packageId;
2512+
} catch (e) {
2513+
return { refused: { handled: true, response: deps.errorFromThrown(e) } };
2514+
}
2515+
return judge(pair);
2516+
},
2517+
};
24802518
}
24812519

24822520
/** The engine methods {@link unregisterAndDeleteFlow} drives. */
@@ -3761,9 +3799,13 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
37613799
}
37623800

37633801
// [#28149, ADR-0146 D3/D4] WHERE the copy lands — never without
3764-
// a package (ADR-0070 D5/D6). Resolved after every refusal above
3802+
// a package (ADR-0070 D5/D6). Judged after every refusal above
37653803
// and before anything is registered, so a refused target leaves
3766-
// no copy behind (see `resolveCloneTarget`).
3804+
// no copy behind; the default pair is ENSURED only once the
3805+
// engine has accepted the copy (`target.land`, asked by
3806+
// `registerAndSaveFlow`), so a copy the engine refuses — an
3807+
// illegal name among it — installs no pair [#29016] (see
3808+
// `resolveCloneTarget`).
37673809
const target = await resolveCloneTarget(deps, context, name, requestedPackage);
37683810
if ('refused' in target) return target.refused;
37693811

@@ -3793,7 +3835,7 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
37933835
// deployment can no longer register. The target name is new (the
37943836
// collision probe above), so a save that fails withdraws it.
37953837
const written = await registerAndSaveFlow(
3796-
deps, context, automationService as FlowRegistrationService, targetName, clone, 'clone', target.packageId,
3838+
deps, context, automationService as FlowRegistrationService, targetName, clone, 'clone', target.land,
37973839
);
37983840
if ('refused' in written) return written.refused;
37993841
// ⛔ NO ANCESTRY on the way out either (ADR-0126 amendment
@@ -3812,7 +3854,7 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
38123854
// came from, so it records no lineage.
38133855
return {
38143856
handled: true,
3815-
response: deps.success({ flow: servedFlowDefinition(clone), packageId: target.packageId, notice: FLOW_CLONE_NOTICE }),
3857+
response: deps.success({ flow: servedFlowDefinition(clone), packageId: written.packageId, notice: FLOW_CLONE_NOTICE }),
38163858
};
38173859
}
38183860
}

0 commit comments

Comments
 (0)