Skip to content

Commit 4b5ec6e

Browse files
os-zhuangclaude
andauthored
fix(automation): re-bind scheduled-flow jobs on os dev hot-reload (#2313)
Editing a schedule-triggered flow under `objectstack dev` silently kept firing the OLD definition until a full server restart. The dev watcher recompiles dist/objectstack.json and MetadataPlugin reloads it into the MetadataManager (so GET /meta + UI HMR are fresh), but the AutomationEngine pulls its flow definitions and trigger/job bindings ONCE at boot — nothing re-registered them on reload. So the scheduled job bound at boot kept running the pre-edit flow (old runAs/schedule/logic) on its timer, with no signal the edit had no effect. MetadataPlugin now fires a generic `metadata:reloaded` hook after each artifact reload (the HMR POST handler and the server-side artifact-file watcher; never on the initial boot load). AutomationServicePlugin subscribes and re-syncs the engine from the metadata service — re-registering every current flow (idempotent: registerFlow re-binds the trigger, ScheduleTrigger.start cancels + reschedules the job) and unregistering flows removed from the artifact so their jobs stop. Covers all auto-triggered flow types (schedule / record-change / api), since record-change flows were also executing boot-time definitions after an edit. Production deployments are unaffected — nothing reloads the artifact there. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 4845c12 commit 4b5ec6e

5 files changed

Lines changed: 423 additions & 2 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
"@objectstack/metadata": patch
4+
---
5+
6+
fix(automation): re-bind scheduled-flow jobs on `os dev` hot-reload
7+
8+
Editing a schedule-triggered flow under `objectstack dev` silently kept firing
9+
the OLD definition until a full server restart. The dev watcher recompiles
10+
`dist/objectstack.json` and MetadataPlugin reloads it into the MetadataManager
11+
(so GET /meta reads + UI HMR are fresh), but the AutomationEngine pulls its flow
12+
definitions and trigger/job bindings ONCE at boot — nothing re-registered them
13+
on reload. So the scheduled job bound at boot kept running the pre-edit flow
14+
(old `runAs`, schedule, or logic) on its timer, with no signal that the edit had
15+
no effect.
16+
17+
Fix: MetadataPlugin now fires a generic `metadata:reloaded` hook after each
18+
artifact reload (the HMR POST handler and the server-side artifact-file watcher;
19+
never on the initial boot load). AutomationServicePlugin subscribes and re-syncs
20+
the engine from the metadata service — re-registering every current flow
21+
(idempotent: `registerFlow` re-binds the trigger, and `ScheduleTrigger.start`
22+
cancels + reschedules the job) and unregistering flows removed from the artifact
23+
so their jobs stop firing. This covers all auto-triggered flow types
24+
(schedule / record-change / api), not just scheduled ones, since record-change
25+
flows were also executing their boot-time definitions after an edit. Production
26+
deployments are unaffected — nothing reloads the artifact there.
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Regression: the `os dev` artifact-reload path (HMR POST handler + server-side
4+
// artifact-file watcher) must announce a generic `metadata:reloaded` hook AFTER
5+
// re-loading the artifact into the MetadataManager. Runtime consumers that cached
6+
// boot-time metadata re-sync on that signal — notably the automation engine,
7+
// which re-binds flow triggers (incl. scheduled jobs) it pulled once at boot.
8+
// Without the announce, an edited schedule-triggered flow keeps firing its
9+
// pre-edit definition until a full restart.
10+
11+
import { describe, it, expect, vi } from 'vitest';
12+
import { mkdtempSync, writeFileSync } from 'node:fs';
13+
import { tmpdir } from 'node:os';
14+
import { join } from 'node:path';
15+
import { MetadataPlugin } from './plugin';
16+
import type { NodeMetadataManager } from './node-metadata-manager';
17+
18+
function fakeCtx() {
19+
return {
20+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
21+
trigger: vi.fn(async () => {}),
22+
} as any;
23+
}
24+
25+
function writeArtifact(flowName: string): string {
26+
const dir = mkdtempSync(join(tmpdir(), 'os-hmr-'));
27+
const file = join(dir, 'objectstack.json');
28+
const artifact = {
29+
id: 'com.example.test',
30+
name: 'test',
31+
version: '0.0.0',
32+
type: 'app',
33+
scope: 'app',
34+
namespace: 'test',
35+
defaultDatasource: 'memory',
36+
flows: [
37+
{
38+
name: flowName,
39+
label: flowName,
40+
type: 'schedule',
41+
runAs: 'system',
42+
nodes: [
43+
{ id: 'start', type: 'start', label: 'Start', config: { schedule: { type: 'interval', intervalMs: 1000 } } },
44+
{ id: 'end', type: 'end', label: 'End' },
45+
],
46+
edges: [{ id: 'e1', source: 'start', target: 'end' }],
47+
},
48+
],
49+
};
50+
writeFileSync(file, JSON.stringify(artifact), 'utf8');
51+
return file;
52+
}
53+
54+
describe('MetadataPlugin._reloadAndAnnounce — fires metadata:reloaded after reload', () => {
55+
it('reloads the artifact into the manager THEN announces metadata:reloaded', async () => {
56+
const plugin = new MetadataPlugin({
57+
watch: false,
58+
config: { bootstrap: 'eager' },
59+
environmentId: 'proj_test',
60+
});
61+
const mgr = (plugin as any).manager as NodeMetadataManager;
62+
const ctx = fakeCtx();
63+
const file = writeArtifact('sweep');
64+
65+
await (plugin as any)._reloadAndAnnounce(ctx, { path: file, fetchTimeoutMs: undefined }, [file]);
66+
67+
// The fresh flow landed in the metadata manager (so the re-sync's
68+
// metadata.list('flow') will see the edited definition)…
69+
const registered = await mgr.get('flow', 'sweep');
70+
expect(registered).toBeDefined();
71+
72+
// …and the generic reload signal fired with the changed path.
73+
expect(ctx.trigger).toHaveBeenCalledTimes(1);
74+
expect(ctx.trigger).toHaveBeenCalledWith('metadata:reloaded', { changed: [file] });
75+
});
76+
77+
it('still announces even if a subscriber throws (reload must not break)', async () => {
78+
const plugin = new MetadataPlugin({
79+
watch: false,
80+
config: { bootstrap: 'eager' },
81+
environmentId: 'proj_test',
82+
});
83+
const ctx = fakeCtx();
84+
ctx.trigger = vi.fn(async () => { throw new Error('subscriber boom'); });
85+
const file = writeArtifact('sweep2');
86+
87+
await expect(
88+
(plugin as any)._reloadAndAnnounce(ctx, { path: file, fetchTimeoutMs: undefined }, [file]),
89+
).resolves.toBeUndefined();
90+
91+
expect(ctx.trigger).toHaveBeenCalledTimes(1);
92+
expect(ctx.logger.warn).toHaveBeenCalled();
93+
});
94+
});

packages/metadata/src/plugin.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ export class MetadataPlugin implements Plugin {
354354
const src = this.options.artifactSource;
355355
if (src?.mode === 'local-file') {
356356
try {
357-
await this._loadFromLocalFile(ctx, src.path, src.fetchTimeoutMs);
357+
await this._reloadAndAnnounce(ctx, src, body?.changed ?? [src.path]);
358358
ctx.logger.info('[MetadataPlugin] artifact reloaded via HMR POST', {
359359
path: src.path,
360360
reason: body?.reason,
@@ -405,7 +405,7 @@ export class MetadataPlugin implements Plugin {
405405
if (pending) return;
406406
pending = true;
407407
try {
408-
await this._loadFromLocalFile(ctx, src.path, src.fetchTimeoutMs);
408+
await this._reloadAndAnnounce(ctx, src, [src.path]);
409409
hub.broadcastReload('artifact-file-changed', [src.path]);
410410
ctx.logger.info('[MetadataPlugin] artifact auto-reloaded (file watcher)', {
411411
path: src.path,
@@ -595,6 +595,32 @@ export class MetadataPlugin implements Plugin {
595595
return totalRegistered;
596596
}
597597

598+
/**
599+
* Reload the artifact from disk into the MetadataManager, then announce a
600+
* generic `metadata:reloaded` hook. Used by BOTH reload paths (the HMR POST
601+
* handler and the server-side artifact-file watcher) — but NOT the initial
602+
* boot load, which other plugins already consume directly.
603+
*
604+
* Runtime consumers that cached boot-time metadata re-sync on this signal.
605+
* The automation engine subscribes to re-bind flow triggers it pulled ONCE
606+
* at boot — notably scheduled jobs: without this, an edited
607+
* schedule-triggered flow keeps firing its pre-edit definition (old runAs /
608+
* schedule / logic) until a full process restart. A subscriber failure is
609+
* logged but never blocks the reload.
610+
*/
611+
private async _reloadAndAnnounce(
612+
ctx: PluginContext,
613+
src: { path: string; fetchTimeoutMs?: number },
614+
changed: string[],
615+
): Promise<void> {
616+
await this._loadFromLocalFile(ctx, src.path, src.fetchTimeoutMs);
617+
try {
618+
await ctx.trigger('metadata:reloaded', { changed });
619+
} catch (e: any) {
620+
ctx.logger.warn('[MetadataPlugin] metadata:reloaded subscriber failed', { error: e?.message });
621+
}
622+
}
623+
598624
private async _loadFromLocalFile(ctx: PluginContext, filePath: string, fetchTimeoutMs?: number): Promise<void> {
599625
const isUrl = /^https?:\/\//i.test(filePath);
600626
ctx.logger.info(
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Regression: `os dev` recompiles dist/objectstack.json on a src edit, and
4+
// MetadataPlugin reloads it into the metadata service + fires 'metadata:reloaded'.
5+
// The automation engine, however, pulled its flow definitions + trigger bindings
6+
// ONCE at boot — so without re-syncing, an edited SCHEDULE-triggered flow keeps
7+
// firing its OLD definition (old runAs / schedule / logic) until a full restart.
8+
//
9+
// This proves the fix end-to-end on the automation side: the 'metadata:reloaded'
10+
// hook re-registers every current flow (re-binding its trigger — for a scheduled
11+
// flow that is the job cancel + reschedule the ScheduleTrigger performs) and
12+
// tears down flows that vanished from the artifact. A recording trigger stands in
13+
// for the concrete ScheduleTrigger (whose idempotent job re-bind is covered by
14+
// trigger-schedule's schedule-runas-e2e test) so this stays dependency-light.
15+
16+
import { describe, it, expect } from 'vitest';
17+
import { LiteKernel } from '@objectstack/core';
18+
import { AutomationEngine } from './engine.js';
19+
import { AutomationServicePlugin } from './plugin.js';
20+
import type { FlowTrigger, FlowTriggerBinding } from './engine.js';
21+
import type { AutomationContext } from '@objectstack/spec/contracts';
22+
23+
const flush = () => new Promise<void>((r) => setTimeout(r, 0));
24+
25+
/** A schedule-triggered flow that touches data, parameterized by runAs + interval. */
26+
function scheduledFlow(name: string, runAs: 'system' | 'user', intervalMs: number) {
27+
return {
28+
name,
29+
label: name,
30+
type: 'schedule',
31+
runAs,
32+
nodes: [
33+
{ id: 'start', type: 'start', label: 'Start', config: { schedule: { type: 'interval', intervalMs } } },
34+
{ id: 'mk', type: 'create_record', label: 'Create', config: { objectName: 'thing', fields: { a: 1 } } },
35+
{ id: 'end', type: 'end', label: 'End' },
36+
],
37+
edges: [
38+
{ id: 'e1', source: 'start', target: 'mk' },
39+
{ id: 'e2', source: 'mk', target: 'end' },
40+
],
41+
};
42+
}
43+
44+
/**
45+
* A FlowTrigger of type 'schedule' that records start/stop and, like the real
46+
* ScheduleTrigger, keeps exactly one binding per flow (re-bind drops the prior).
47+
* `fire()` invokes the engine callback the way a cron tick would.
48+
*/
49+
function recordingScheduleTrigger() {
50+
const bound = new Map<string, { schedule: any; cb: (ctx: AutomationContext) => Promise<void> }>();
51+
const events: Array<{ op: 'start' | 'stop'; flow: string }> = [];
52+
const trigger: FlowTrigger = {
53+
type: 'schedule',
54+
start(binding: FlowTriggerBinding, cb: (ctx: AutomationContext) => Promise<void>) {
55+
const schedule = binding.schedule ?? (binding.config as any)?.schedule;
56+
bound.set(binding.flowName, { schedule, cb });
57+
events.push({ op: 'start', flow: binding.flowName });
58+
},
59+
stop(flowName: string) {
60+
if (bound.delete(flowName)) events.push({ op: 'stop', flow: flowName });
61+
},
62+
};
63+
return {
64+
trigger,
65+
has: (n: string) => bound.has(n),
66+
intervalOf: (n: string) => bound.get(n)?.schedule?.intervalMs,
67+
fire: async (n: string) => {
68+
await bound.get(n)?.cb({ event: 'schedule', params: { flowName: n } } as AutomationContext);
69+
},
70+
events,
71+
};
72+
}
73+
74+
/** Stub `create_record` executor that captures the runAs it is handed. */
75+
function captureRunAs(engine: AutomationEngine): Array<string | undefined> {
76+
const seen: Array<string | undefined> = [];
77+
engine.registerNodeExecutor({
78+
type: 'create_record',
79+
async execute(_node: unknown, _vars: unknown, context: AutomationContext) {
80+
seen.push(context.runAs);
81+
return { success: true, output: {} };
82+
},
83+
} as never);
84+
return seen;
85+
}
86+
87+
/** A mutable fake `metadata` service exposing just the `list('flow')` the re-sync uses. */
88+
function fakeMetadataService(initial: unknown[]) {
89+
let flows = initial;
90+
return {
91+
service: {
92+
async list(type: string) {
93+
return type === 'flow' ? flows : [];
94+
},
95+
},
96+
setFlows: (next: unknown[]) => { flows = next; },
97+
};
98+
}
99+
100+
async function bootKernel(meta: { service: unknown }) {
101+
const kernel = new LiteKernel({ logger: { level: 'silent' } } as never);
102+
const harness = {
103+
name: 'test.harness',
104+
type: 'standard' as const,
105+
version: '1.0.0',
106+
dependencies: [] as string[],
107+
async init(ctx: any) {
108+
ctx.registerService('metadata', meta.service);
109+
},
110+
async start() {},
111+
};
112+
kernel.use(harness as never);
113+
kernel.use(new AutomationServicePlugin());
114+
await kernel.bootstrap();
115+
return kernel;
116+
}
117+
118+
const reload = (kernel: LiteKernel) => (kernel as any).context.trigger('metadata:reloaded', {});
119+
120+
describe("scheduled flow hot-reload re-bind (metadata:reloaded re-sync)", () => {
121+
it('re-binds an edited scheduled flow to its NEW definition without a restart', async () => {
122+
const meta = fakeMetadataService([scheduledFlow('sweep', 'user', 1000)]);
123+
const kernel = await bootKernel(meta);
124+
const engine = kernel.getService<AutomationEngine>('automation');
125+
const seen = captureRunAs(engine);
126+
const sched = recordingScheduleTrigger();
127+
engine.registerTrigger(sched.trigger);
128+
129+
// First reload binds v1 (runAs:user, interval 1000) — like the boot bind.
130+
await reload(kernel);
131+
await flush();
132+
expect(sched.has('sweep'), 'flow bound to the schedule trigger').toBe(true);
133+
expect(sched.intervalOf('sweep')).toBe(1000);
134+
await sched.fire('sweep');
135+
expect(seen[seen.length - 1], 'v1 runs as user').toBe('user');
136+
137+
// Edit the flow: runAs:system + interval 5000. Recompile → reload.
138+
sched.events.length = 0;
139+
meta.setFlows([scheduledFlow('sweep', 'system', 5000)]);
140+
await reload(kernel);
141+
await flush();
142+
143+
// The OLD binding was torn down and a NEW one created (not left stale).
144+
expect(sched.events).toEqual([
145+
{ op: 'stop', flow: 'sweep' },
146+
{ op: 'start', flow: 'sweep' },
147+
]);
148+
expect(sched.intervalOf('sweep'), 'schedule re-bound to the new interval').toBe(5000);
149+
150+
// The fired job now runs the NEW definition — the actual footgun fixed.
151+
await sched.fire('sweep');
152+
expect(seen[seen.length - 1], 'v2 runs elevated as system after reload').toBe('system');
153+
154+
await kernel.shutdown();
155+
});
156+
157+
it('tears down a scheduled flow that was deleted from the artifact', async () => {
158+
const meta = fakeMetadataService([scheduledFlow('sweep', 'user', 1000)]);
159+
const kernel = await bootKernel(meta);
160+
const engine = kernel.getService<AutomationEngine>('automation');
161+
const sched = recordingScheduleTrigger();
162+
engine.registerTrigger(sched.trigger);
163+
164+
await reload(kernel);
165+
await flush();
166+
expect(sched.has('sweep')).toBe(true);
167+
168+
// Delete the flow file → recompiled artifact no longer carries it.
169+
meta.setFlows([]);
170+
await reload(kernel);
171+
await flush();
172+
173+
expect(sched.has('sweep'), 'deleted flow unbound — its job stops firing').toBe(false);
174+
expect(await engine.listFlows()).not.toContain('sweep');
175+
176+
await kernel.shutdown();
177+
});
178+
});

0 commit comments

Comments
 (0)