Skip to content

Commit 063f960

Browse files
committed
wip(automation, ui): a resumed parked run seeds the tenant default, a shared refusal no longer blames the translation, action copy logs its refusal
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGufzizxYz7JyC5JLW3Gmv
1 parent 9b2bab6 commit 063f960

7 files changed

Lines changed: 176 additions & 9 deletions

File tree

‎packages/services/service-automation/src/builtin/notify-currency-tenant-default.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
import { beforeEach, describe, expect, it } from 'vitest';
1414
import { AutomationEngine } from '../engine.js';
15+
import { InMemorySuspendedRunStore } from '../suspended-run-store.js';
16+
import { installBuiltinNodes } from './index.js';
1517
import { registerNotifyNode } from './notify-node.js';
1618
import type { MessagingServiceSurface } from './notify-node.js';
1719
import { FlowTextTemplateError, renderTextSlot } from './template.js';
@@ -108,6 +110,57 @@ describe('notify — a no-argument currency hole formats in the run organization
108110
});
109111
});
110112

113+
describe('a run parked before run setup seeded a currency resumes in the organization\'s default', () => {
114+
const PARKED = {
115+
name: 'deal_review',
116+
label: 'Deal Review',
117+
type: 'screen' as const,
118+
variables: [{ name: 'amount', type: 'number' as const, isInput: true }],
119+
nodes: [
120+
{ id: 'start', type: 'start' as const, label: 'Start' },
121+
{ id: 'ask', type: 'screen' as const, label: 'Ask', config: { waitForInput: true, title: 'Confirm', fields: [{ name: 'ok', label: 'OK', type: 'checkbox' }] } },
122+
{ id: 'notify', type: 'notify' as const, label: 'Notify', config: { recipients: ['user_1'], title: 'Won {{ amount | currency }}', message: 'x' } },
123+
{ id: 'end', type: 'end' as const, label: 'End' },
124+
],
125+
edges: [
126+
{ id: 'e1', source: 'start', target: 'ask' },
127+
{ id: 'e2', source: 'ask', target: 'notify' },
128+
{ id: 'e3', source: 'notify', target: 'end' },
129+
],
130+
};
131+
132+
function engineOver(store: InMemorySuspendedRunStore, messaging: ReturnType<typeof fakeMessaging>) {
133+
const engine = new AutomationEngine(createTestLogger(), store);
134+
installBuiltinNodes(engine, {
135+
logger: createTestLogger(),
136+
getService: (name: string) => (name === 'messaging' ? messaging.service : undefined),
137+
} as never);
138+
engine.registerFlow('deal_review', PARKED);
139+
return engine;
140+
}
141+
142+
it('a cold resume seeds the default the persisted context lacks, and leaves the persisted context as it parked', async () => {
143+
const store = new InMemorySuspendedRunStore();
144+
const before = engineOver(store, fakeMessaging());
145+
// Parked by an engine with no resolver: the persisted context carries no currency.
146+
const paused = await before.execute('deal_review', { params: { amount: 1234.5 }, tenantId: 'org_cn' } as any);
147+
expect(paused.status).toBe('paused');
148+
const parked = await store.load(paused.runId!);
149+
expect(parked?.context).not.toHaveProperty('currency');
150+
151+
const messaging = fakeMessaging();
152+
const after = engineOver(store, messaging);
153+
const reads: Array<string | undefined> = [];
154+
after.setTenantCurrencyResolver((tenantId) => { reads.push(tenantId); return 'CNY'; });
155+
const resumed = await after.resume(paused.runId!, { variables: { ok: true } } as never);
156+
157+
expect(resumed.success, JSON.stringify(resumed)).toBe(true);
158+
expect(reads).toEqual(['org_cn']);
159+
expect(messaging.emitted[0]?.payload).toMatchObject({ title: 'Won CN¥1,234.50' });
160+
expect(parked?.context).not.toHaveProperty('currency');
161+
});
162+
});
163+
111164
describe('renderTextSlot — the tenant default on the engine\'s reserved channel', () => {
112165
const vars = new Map<string, unknown>([['amount', 1234.5], ['currency', 'EUR']]);
113166

‎packages/services/service-automation/src/end-node-refusal-translation.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,40 @@ describe('#22450 — the authored message renders whenever no translation is pic
163163
});
164164
});
165165

166+
describe('a refusal both texts share is the authored text\'s, and no line blames the translation', () => {
167+
it('a no-argument `currency` hole with no tenant default fails the run naming the hole, with no translation warning', async () => {
168+
const lines: LogLine[] = [];
169+
const zh = { flows: { dedupe: { refusals: { finish: { message: '已拒绝:{{ record.amount | currency }}' } } } } };
170+
const { engine } = engineWith(i18nWith({ 'zh-CN': zh }), lines);
171+
const flow = refusingFlow();
172+
(flow.nodes[1] as { config: { message: string } }).config.message = 'Refused: {{ record.amount | currency }}';
173+
engine.registerFlow('dedupe', flow as never);
174+
175+
const result = await engine.execute('dedupe', {
176+
...doorContext('zh-CN'), record: { ...ACME, amount: 1234.5 },
177+
} as unknown as AutomationContext);
178+
179+
expect(result.success).toBe(false);
180+
expect(String(result.error)).toContain('{{ record.amount | currency }}');
181+
expect(String(result.error)).toContain('localization.currency');
182+
expect(lines.filter((l) => l.level === 'warn' && l.message.includes('translation'))).toEqual([]);
183+
});
184+
185+
it('CONTROL — with the tenant default on the run, the zh-CN translation renders in it', async () => {
186+
const zh = { flows: { dedupe: { refusals: { finish: { message: '已拒绝:{{ record.amount | currency }}' } } } } };
187+
const { engine } = engineWith(i18nWith({ 'zh-CN': zh }));
188+
const flow = refusingFlow();
189+
(flow.nodes[1] as { config: { message: string } }).config.message = 'Refused: {{ record.amount | currency }}';
190+
engine.registerFlow('dedupe', flow as never);
191+
192+
const result = await engine.execute('dedupe', {
193+
...doorContext('zh-CN'), record: { ...ACME, amount: 1234.5 }, currency: 'CNY',
194+
} as unknown as AutomationContext);
195+
196+
expect(result.refusalMessage).toBe(`已拒绝:${new Intl.NumberFormat('en-US', { style: 'currency', currency: 'CNY' }).format(1234.5)}`);
197+
});
198+
});
199+
166200
describe('#22450 — a door-started run carries its locale into a resumed leg', () => {
167201
/** `start → ask (screen) → finish (refused)`: the refusal renders on the RESUMED leg. */
168202
function reviewFlow() {

‎packages/services/service-automation/src/engine.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5636,6 +5636,11 @@ export class AutomationEngine implements IAutomationService {
56365636
* permission sets, so its data operations fall back to the baseline member
56375637
* set — narrower, never the trigger user's grants (see the warning in
56385638
* {@link seedRunGrants}).
5639+
*
5640+
* A segment whose persisted context carries no
5641+
* {@link AutomationContext.currency} — a run parked before run setup
5642+
* seeded one — is seeded here the way {@link resolveRunContext} seeds a
5643+
* new run, on a copy: the persisted `run.context` is left as it parked.
56395644
*/
56405645
private resolveDecisionSegmentContext(
56415646
run: SuspendedRun,
@@ -5644,6 +5649,7 @@ export class AutomationEngine implements IAutomationService {
56445649
// The body lives in ./run-resume.ts (#25222).
56455650
const host: ThisParameterType<typeof resolveDecisionSegmentContext> = {
56465651
seedRunGrants: (...a) => this.seedRunGrants(...a),
5652+
tenantCurrencyResolver: this.tenantCurrencyResolver,
56475653
};
56485654
return resolveDecisionSegmentContext.call(host, run, signal);
56495655
}

‎packages/services/service-automation/src/node-text-rendering.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,27 +113,36 @@ export function renderRefusalMessage(
113113
);
114114
}
115115

116-
/** The body of {@link AutomationEngine.renderFlowTextSlot}, moved unedited; that method's docblock is its contract. */
116+
/** The body of {@link AutomationEngine.renderFlowTextSlot}; that method's docblock is its contract. */
117117
export function renderFlowTextSlot(
118118
this: Pick<NodeTextRenderingHost, 'translatedFlowTemplate' | 'logger'>,
119119
slot: FlowTextSlotTranslation,
120120
variables: Map<string, unknown>,
121121
context: AutomationContext,
122122
): string | undefined {
123123
const translated = this.translatedFlowTemplate(slot, context.locale);
124+
let translationRefusal: FlowTextTemplateError | undefined;
124125
if (translated !== undefined) {
125126
try {
126127
return renderTextSlot(translated.template, variables, context.currency);
127128
} catch (err: unknown) {
128129
if (!(err instanceof FlowTextTemplateError)) throw err;
129-
this.logger.warn(
130-
`[automation] ${slot.subject}: the '${translated.locale}' translation at '${slot.key}' does not ` +
131-
`compile (${err.message}), so ${slot.shownAs} shows its authored text instead. Fix the ` +
132-
`translation's holes to match the authored text.`,
133-
);
130+
translationRefusal = err;
134131
}
135132
}
136-
return renderTextSlot(slot.authored, variables, context.currency);
133+
// The authored text renders before the translation is blamed: when it
134+
// throws too (a `{{ amount | currency }}` hole that meets no tenant
135+
// default refuses both alike), its own refusal is the answer, and no line
136+
// tells anyone to fix a translation that is not at fault.
137+
const authored = renderTextSlot(slot.authored, variables, context.currency);
138+
if (translated !== undefined && translationRefusal !== undefined) {
139+
this.logger.warn(
140+
`[automation] ${slot.subject}: the '${translated.locale}' translation at '${slot.key}' does not ` +
141+
`compile (${translationRefusal.message}), so ${slot.shownAs} shows its authored text instead. Fix the ` +
142+
`translation's holes to match the authored text.`,
143+
);
144+
}
145+
return authored;
137146
}
138147

139148
/** The body of {@link AutomationEngine.translatedFlowTemplate}, moved unedited; that method's docblock is its contract. */

‎packages/services/service-automation/src/run-resume.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,37 @@ export async function seedRunGrants(
262262
}
263263
}
264264

265-
/** The body of {@link AutomationEngine.resolveDecisionSegmentContext}, moved unedited; that method's docblock is its contract. */
265+
/** The body of {@link AutomationEngine.resolveDecisionSegmentContext}; that method's docblock is its contract. */
266266
export async function resolveDecisionSegmentContext(
267+
this: Pick<RunResumeHost, 'seedRunGrants' | 'tenantCurrencyResolver'>,
268+
run: SuspendedRun,
269+
signal: ResumeSignal,
270+
): Promise<AutomationContext> {
271+
return withTenantCurrency.call(this, await deciderSegmentContext.call(this, run, signal));
272+
}
273+
274+
/**
275+
* A resumed segment's context with the organization's default currency, when
276+
* the persisted one carries none: a run parked before run setup seeded
277+
* `currency` would otherwise refuse a `{{ amount | currency }}` hole whose
278+
* remedy, the setting, is already in place. A copy, never the persisted
279+
* context; a failed or empty read leaves it as it was.
280+
*/
281+
async function withTenantCurrency(
282+
this: Pick<RunResumeHost, 'tenantCurrencyResolver'>,
283+
context: AutomationContext,
284+
): Promise<AutomationContext> {
285+
if (context?.currency !== undefined || !this.tenantCurrencyResolver) return context;
286+
try {
287+
const currency = await this.tenantCurrencyResolver(context?.tenantId);
288+
if (typeof currency === 'string' && currency !== '') return { ...(context ?? {}), currency } as AutomationContext;
289+
} catch {
290+
// Tolerated, as at run setup: the hole's own refusal is the loud answer.
291+
}
292+
return context;
293+
}
294+
295+
async function deciderSegmentContext(
267296
this: Pick<RunResumeHost, 'seedRunGrants'>,
268297
run: SuspendedRun,
269298
signal: ResumeSignal,

‎ui/packages/core/src/actions/__tests__/ActionRunner.copyTemplateTenantCurrency-26939.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,23 @@ describe('fillTemplateHoles: a no-argument currency hole reads the tenant defaul
4646
expect(fillTemplateHoles(source, { amount: AMOUNT }, 'en-US')).toBe(source);
4747
});
4848

49+
it('a refused sentence logs the refusal, naming the hole and the remedy, once', () => {
50+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
51+
try {
52+
const source = 'Logged {{ amount | currency }}?';
53+
expect(fillTemplateHoles(source, { amount: AMOUNT }, 'en-US')).toBe(source);
54+
expect(fillTemplateHoles(source, { amount: AMOUNT }, 'en-US')).toBe(source);
55+
expect(warn).toHaveBeenCalledTimes(1);
56+
const line = String(warn.mock.calls[0][0]);
57+
expect(line).toContain(source);
58+
expect(line).toContain('{{ amount | currency }}');
59+
expect(line).toContain('localization.currency');
60+
expect(line).toContain('currency:EUR');
61+
} finally {
62+
warn.mockRestore();
63+
}
64+
});
65+
4966
it('a hole that names its code needs no tenant default', () => {
5067
expect(fillTemplateHoles('{{ amount | currency:EUR }}', { amount: AMOUNT }, 'en-US')).toBe(money('en-US', 'EUR'));
5168
});

‎ui/packages/core/src/actions/actionCopyTemplate.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ export function hasTemplateHoles(text: string): boolean {
9090
* channel: a `{{ amount | currency }}` hole with no argument formats in it.
9191
* Omitted, the engine refuses such a sentence and it is returned as written,
9292
* like any sentence the engine refuses — never with a guessed currency.
93+
*
94+
* A refused sentence is shown as written, and the engine's refusal — which
95+
* names the hole and the remedy (for a missing default: set
96+
* `localization.currency`, or write `currency:CODE`) — goes to
97+
* `console.warn`, once per distinct refusal, so it reaches whoever reads the
98+
* console rather than nobody.
9399
*/
94100
export function fillTemplateHoles(
95101
source: string,
@@ -101,5 +107,18 @@ export function fillTemplateHoles(
101107
{ dialect: 'template', source },
102108
{ extra: roots, locale, ...(currency ? { currency } : {}) },
103109
);
104-
return filled.ok && typeof filled.value === 'string' ? filled.value : source;
110+
if (filled.ok && typeof filled.value === 'string') return filled.value;
111+
if (!filled.ok) warnRefusedCopy(source, filled.error.message);
112+
return source;
113+
}
114+
115+
/** Refusals already logged, so a dialog that re-renders does not repeat one. */
116+
const warnedRefusals = new Set<string>();
117+
118+
function warnRefusedCopy(source: string, refusal: string): void {
119+
const key = `${source}\u0000${refusal}`;
120+
if (warnedRefusals.has(key)) return;
121+
if (warnedRefusals.size >= 200) warnedRefusals.clear();
122+
warnedRefusals.add(key);
123+
console.warn(`[action copy] the sentence "${source}" is shown as written: ${refusal}`);
105124
}

0 commit comments

Comments
 (0)