From 1a7ce83b7066ccc062043767876b8620bbe9a820 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 18:11:18 +0000 Subject: [PATCH 1/2] test(service-automation): pin that a trigger registered after ledger hydration leaves a disabled flow unbound (red on main) The pins drive every arming path from outside: a trigger type registered after hydrateFlowActivations(), a cold restart over one activation store, the enable toggle, and the trigger-fired failure line's run-history claim. Ten of eleven are red on the unfixed engine; the dispatched-and-failed control is green. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude --- .../src/flow-activation-late-trigger.test.ts | 290 ++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 packages/services/service-automation/src/flow-activation-late-trigger.test.ts diff --git a/packages/services/service-automation/src/flow-activation-late-trigger.test.ts b/packages/services/service-automation/src/flow-activation-late-trigger.test.ts new file mode 100644 index 00000000000..b1738830535 --- /dev/null +++ b/packages/services/service-automation/src/flow-activation-late-trigger.test.ts @@ -0,0 +1,290 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#20677] ADR-0126 §7.2 — a switched-off flow stays UNBOUND whenever its +// trigger type arrives, and a restart does not re-arm it. +// +// ## The boot order this pins +// +// `AutomationServicePlugin.start()` pulls the flows (`registerFlow`) and then +// applies the activation ledger (`hydrateFlowActivations()`), which unbinds +// every flow a row switches off. The trigger plugins register their triggers +// LATER, at `kernel:ready` — and `registerTrigger` arms every registered, +// unbound flow whose trigger type matches. Before the fix it did so without +// asking whether the flow may run at all, so a flow the administrator switched +// off came back `bound: true` after every cold boot, and each matching event +// then logged an ERROR claiming a run-history row that was never written. +// +// ## The shape of the fix these pins hold +// +// ONE gate, in `activateFlowTrigger` itself: it refuses to arm a flow +// `isFlowEnabled` answers `false` for, so `registerFlow`, `registerTrigger`, +// the enable toggle and any arming path added later all inherit it. The pins +// therefore drive the arming paths from outside rather than asserting on +// `registerTrigger`'s body. + +import { describe, it, expect, vi } from 'vitest'; +import { AutomationEngine } from './engine.js'; +import type { FlowTrigger, FlowTriggerBinding, FlowActivationStore } from './engine.js'; +import { InMemoryFlowActivationStore } from './flow-activation-store.js'; +import type { AutomationContext } from '@objectstack/spec/contracts'; +import { withScheduledWorkOn } from './deployment-switch.test-support.js'; + +function createTestLogger(): any { + const l: any = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + l.child = () => l; + return l; +} + +/** A packaged, runnable flow whose `start` config decides its trigger kind. */ +function packagedFlow(name: string, startConfig: Record, extra: Record = {}) { + return { + name, + label: name, + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: startConfig }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + _packageId: 'crm', + ...extra, + }; +} + +/** A recording trigger: binding state is read off the trigger, never inferred. */ +function recordingTrigger(type: string) { + const bound = new Map Promise>(); + const trigger: FlowTrigger = { + type, + start(binding: FlowTriggerBinding, cb: (ctx: AutomationContext) => Promise) { + bound.set(binding.flowName, cb); + }, + stop(flowName: string) { + bound.delete(flowName); + }, + }; + return { trigger, isBound: (n: string) => bound.has(n), callbackOf: (n: string) => bound.get(n) }; +} + +/** The runtime-state row `GET /automation/_status` serves for one flow. */ +function stateOf(engine: AutomationEngine, name: string) { + const row = engine.getFlowRuntimeStates().find((s) => s.name === name); + return row && { enabled: row.enabled, bound: row.bound }; +} + +const RECORD_CHANGE = { objectName: 'task', triggerType: 'record-after-create' }; + +// Time-triggered kinds arm only where the deployment runs package-authored +// scheduled work (off by default); without this every schedule / time-relative +// assertion below would fail for a reason unrelated to the ledger. +withScheduledWorkOn(); + +describe('a trigger type registered AFTER hydrateFlowActivations() does not arm a ledger-disabled flow', () => { + // Every kind a trigger plugin registers at `kernel:ready`. The disabled + // flow and its enabled sibling share the kind, so the sibling is the + // control that the trigger registration really did arm what it should. + const kinds: Array<[string, Record]> = [ + ['record_change', RECORD_CHANGE], + ['schedule', { schedule: '0 9 * * *' }], + ['time_relative', { timeRelative: { object: 'task', field: 'due_at' }, schedule: '0 * * * *' }], + // An `api` flow registers only with its per-flow secret (ADR-0041). + ['api', { triggerType: 'api', secret: 'hook-secret' }], + ]; + + for (const [kind, startConfig] of kinds) { + it(`${kind}: the switched-off flow stays unbound, its enabled sibling arms`, async () => { + const store = new InMemoryFlowActivationStore(); + await store.setActive({ name: 'off', packageId: 'crm', active: false }); + + const engine = new AutomationEngine(createTestLogger()); + engine.setFlowActivationStore(store); + engine.registerFlow('off', packagedFlow('off', startConfig)); + engine.registerFlow('on', packagedFlow('on', startConfig)); + expect(await engine.hydrateFlowActivations()).toEqual(['off']); + + // The trigger plugin's `kernel:ready` registration, after hydration. + const trigger = recordingTrigger(kind); + engine.registerTrigger(trigger.trigger); + + expect(trigger.isBound('off')).toBe(false); + expect(stateOf(engine, 'off')).toEqual({ enabled: false, bound: false }); + // The control: the same registration armed the enabled sibling. + expect(trigger.isBound('on')).toBe(true); + expect(stateOf(engine, 'on')).toEqual({ enabled: true, bound: true }); + }); + } + + it('a STATUS-disabled flow is not armed by a later trigger registration either — the gate is `isFlowEnabled`, both dimensions', async () => { + const engine = new AutomationEngine(createTestLogger()); + engine.registerFlow('retired', packagedFlow('retired', RECORD_CHANGE, { status: 'obsolete' })); + engine.registerFlow('live', packagedFlow('live', RECORD_CHANGE, { status: 'active' })); + + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); + + expect(trigger.isBound('retired')).toBe(false); + expect(stateOf(engine, 'retired')).toEqual({ enabled: false, bound: false }); + expect(trigger.isBound('live')).toBe(true); + }); +}); + +describe('the enable path still arms', () => { + it('re-enabling a flow hydrated as disabled arms it on the trigger that registered after hydration', async () => { + const store = new InMemoryFlowActivationStore(); + await store.setActive({ name: 'f', packageId: 'crm', active: false }); + const engine = new AutomationEngine(createTestLogger()); + engine.setFlowActivationStore(store); + engine.registerFlow('f', packagedFlow('f', RECORD_CHANGE)); + await engine.hydrateFlowActivations(); + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); + expect(trigger.isBound('f')).toBe(false); + + await engine.toggleFlow('f', true); + + expect(trigger.isBound('f')).toBe(true); + expect(stateOf(engine, 'f')).toEqual({ enabled: true, bound: true }); + expect(await store.list()).toEqual([{ name: 'f', packageId: 'crm', active: true }]); + expect((await engine.execute('f')).success).toBe(true); + }); + + it('re-enabling the LEDGER bit of a flow whose STATUS is obsolete does not arm it — it still cannot run', async () => { + const engine = new AutomationEngine(createTestLogger()); + engine.setFlowActivationStore(new InMemoryFlowActivationStore()); + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); + engine.registerFlow('retired', packagedFlow('retired', RECORD_CHANGE, { status: 'obsolete' })); + + await engine.toggleFlow('retired', true); + + // Armed, it would refuse every event it fired (`FLOW_DISABLED`, the + // status dimension) — the same armed-but-cannot-run state as a + // ledger-disabled flow, reached through the toggle instead. + expect(trigger.isBound('retired')).toBe(false); + expect(stateOf(engine, 'retired')).toEqual({ enabled: false, bound: false }); + expect((await engine.execute('retired')).code).toBe('FLOW_DISABLED'); + }); +}); + +describe('a cold restart over the same store keeps a switched-off flow enabled:false, bound:false', () => { + /** + * One process lifetime, in `AutomationServicePlugin`'s boot order: + * `start()` pulls the flows and hydrates the ledger; at `kernel:ready` the + * protocol sync re-registers every flow and the trigger plugin registers + * its trigger; `kernel:bootstrapped` seals the node-type vocabulary. Only + * the STORE outlives the engine, as the `sys_metadata_activation` rows + * outlive the process. + */ + async function boot(store: FlowActivationStore) { + const engine = new AutomationEngine(createTestLogger()); + engine.setFlowActivationStore(store); + const defs = [packagedFlow('urgent_alert', RECORD_CHANGE), packagedFlow('welcome', RECORD_CHANGE)]; + for (const def of defs) engine.registerFlow(def.name, def); // the boot pull + await engine.hydrateFlowActivations(); + for (const def of defs) engine.registerFlow(def.name, def); // kernel:ready protocol sync + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); // kernel:ready trigger plugin + engine.sealNodeTypeVocabulary(); // kernel:bootstrapped + return { engine, trigger }; + } + + it('survives two consecutive cold boots; the enabled sibling arms on each', async () => { + const store = new InMemoryFlowActivationStore(); + + const first = await boot(store); + expect(stateOf(first.engine, 'urgent_alert')).toEqual({ enabled: true, bound: true }); + await first.engine.toggleFlow('urgent_alert', false); + expect(stateOf(first.engine, 'urgent_alert')).toEqual({ enabled: false, bound: false }); + + for (const restart of [1, 2]) { + const { engine, trigger } = await boot(store); + expect(stateOf(engine, 'urgent_alert'), `restart ${restart}`).toEqual({ enabled: false, bound: false }); + expect(trigger.isBound('urgent_alert'), `restart ${restart}`).toBe(false); + expect(stateOf(engine, 'welcome'), `restart ${restart}`).toEqual({ enabled: true, bound: true }); + // The binding audit reads the same bookkeeping: a disabled flow is + // not reported as a binding failure either. + expect(engine.getTriggerBindingAudit(), `restart ${restart}`).toEqual([]); + } + }); +}); + +describe('the trigger-fired failure line claims a run-history row only when one is written', () => { + const flush = () => new Promise((r) => setTimeout(r, 0)); + + function loggedLines(logger: any, level: 'info' | 'error') { + return logger[level].mock.calls.map((c: any[]) => String(c[0])); + } + + it('a FLOW_DISABLED refusal reaching the callback (an event in flight when the flow was switched off) is not logged as a failure', async () => { + const logger = createTestLogger(); + const engine = new AutomationEngine(logger); + engine.setFlowActivationStore(new InMemoryFlowActivationStore()); + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); + engine.registerFlow('f', packagedFlow('f', RECORD_CHANGE)); + // The callback the trigger already holds — an event dispatched before + // the switch-off is delivered to it after. + const inFlight = trigger.callbackOf('f'); + expect(inFlight).toBeTypeOf('function'); + await engine.toggleFlow('f', false); + + await inFlight!({ event: 'record-after-create', object: 'task', record: { id: 't1' } } as AutomationContext); + await flush(); + + expect(loggedLines(logger, 'error').filter((m: string) => m.includes('Trigger-fired run'))).toEqual([]); + const told = loggedLines(logger, 'info').filter((m: string) => m.includes("flow 'f'") && m.includes('disabled')); + expect(told).toHaveLength(1); + expect(told[0]).not.toContain('recorded in the flow\'s run history'); + // Nothing ran, so nothing may claim a row: there is none. + expect(await engine.listRuns('f')).toEqual([]); + }); + + it('a run that dispatched and failed still logs at ERROR and still says its failure is in the run history — which holds it', async () => { + const logger = createTestLogger(); + const engine = new AutomationEngine(logger); + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); + engine.registerNodeExecutor({ type: 'exploder', async execute() { throw new Error('boom'); } } as never); + engine.registerFlow('failing', { + ...packagedFlow('failing', RECORD_CHANGE), + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: RECORD_CHANGE }, + { id: 'boom', type: 'exploder', label: 'Boom' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'boom' }, + { id: 'e2', source: 'boom', target: 'end' }, + ], + }); + + await trigger.callbackOf('failing')!({ event: 'record-after-create', object: 'task', record: { id: 't1' } } as AutomationContext); + await flush(); + + const errors = loggedLines(logger, 'error').filter((m: string) => m.includes('Trigger-fired run')); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain("recorded in the flow's run history"); + const runs = await engine.listRuns('failing'); + expect(runs.map((r) => r.status)).toEqual(['failed']); + }); + + it('a failure refused before it dispatched makes no run-history claim — no row was written', async () => { + const logger = createTestLogger(); + const engine = new AutomationEngine(logger); + const trigger = recordingTrigger('record_change'); + engine.registerTrigger(trigger.trigger); + engine.registerFlow('gone', packagedFlow('gone', RECORD_CHANGE)); + const inFlight = trigger.callbackOf('gone'); + // Unregistered while an event is in flight: `execute()` answers the + // never-dispatched "not found" exit, which records nothing. + engine.unregisterFlow('gone'); + + await inFlight!({ event: 'record-after-create', object: 'task', record: { id: 't1' } } as AutomationContext); + await flush(); + + const errors = loggedLines(logger, 'error').filter((m: string) => m.includes('Trigger-fired run')); + expect(errors).toHaveLength(1); + expect(errors[0]).not.toContain("recorded in the flow's run history"); + expect(await engine.listRuns('gone')).toEqual([]); + }); +}); From 758967616e1f8078c884bbea146c5980c19e7ad2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 18:13:12 +0000 Subject: [PATCH 2/2] fix(service-automation): one enablement gate in activateFlowTrigger, so a trigger registered after ledger hydration never arms a disabled flow activateFlowTrigger now refuses to arm a flow isFlowEnabled answers false for, so registerFlow, registerTrigger, the enable toggle and any later arming path inherit it; registerFlow's caller-side check is folded into it. The trigger-fired callback says a failure is in the run history only for a run that dispatched and failed (status 'failed'), and a FLOW_DISABLED refusal that still reaches it is logged at info as a refusal, not at error as a failed run. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude --- .../20677-ledger-disabled-stays-unbound.md | 39 ++++++++ .../services/service-automation/src/engine.ts | 91 +++++++++++++++---- 2 files changed, 113 insertions(+), 17 deletions(-) create mode 100644 .changeset/20677-ledger-disabled-stays-unbound.md diff --git a/.changeset/20677-ledger-disabled-stays-unbound.md b/.changeset/20677-ledger-disabled-stays-unbound.md new file mode 100644 index 00000000000..c258c02e503 --- /dev/null +++ b/.changeset/20677-ledger-disabled-stays-unbound.md @@ -0,0 +1,39 @@ +--- +'@objectstack/service-automation': patch +--- + +fix(service-automation): a flow switched off in the activation ledger stays unbound after a restart, and a trigger-fired refusal no longer logs an ERROR claiming a run-history row (#20677) + +Clause-②: no + +**What was wrong.** A packaged flow switched off through the ADR-0126 activation +ledger (`POST /api/v1/automation/:name/toggle` with `enabled: false`) came back +`bound: true` after every cold restart. Its runs were still refused, so the switch +itself held, but its trigger was armed again. At boot the automation service pulls +the flows and applies the ledger, which leaves a switched-off flow unbound. The +trigger plugins register later, at `kernel:ready`, and registering a trigger armed +every matching flow without asking whether it may run. So `GET +/api/v1/automation/_status` reported the flow `enabled: false, bound: true`. Each +matching event also logged `ERROR Trigger-fired run of flow '…' failed`, saying the +failure "is recorded in the flow's run history", while no run row was written. + +**What changed.** + +- The engine checks whether a flow may run in one place: at the step that arms a + trigger. Every arming path goes through it: flow registration (boot pull, + publish, hot reload), trigger registration, and the enable toggle. A flow that + either disable dimension switches off (the activation ledger, or an `obsolete` / + `invalid` status) is never armed, whenever its trigger registers. +- Re-enabling a flow arms it on its trigger as before. Re-enabling the ledger bit + of a flow whose `status` is still `obsolete` or `invalid` no longer arms it, since + every run it fired would be refused. +- A trigger-fired run refused because the flow is disabled (for example, an event + already in flight when the flow was switched off) is logged at `info`, saying + nothing ran and no run-history row records it. It is no longer an `ERROR`. +- The `ERROR` line for any other trigger-fired failure says the failure is + recorded in the run history only for a run that dispatched and failed. A run + refused before it dispatched gets the same line without that claim. + +**What is not affected.** The runtime refusal (`FLOW_DISABLED`) and its message are +unchanged. The enabled flows beside a disabled one arm exactly as before. No export, +option, route or response shape changes. diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index f0a7f62aac1..566ed4d6792 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -3371,6 +3371,10 @@ export class AutomationEngine implements IAutomationService { // A trigger may be registered *after* its flows (e.g. AutomationServicePlugin // pulls flows at start(); a trigger plugin wires up on kernel:ready, which // fires later). Activate any already-registered flow that maps to this type. + // [ADR-0126 §7.2] A flow that may not run is skipped by + // `activateFlowTrigger`'s own enablement gate, so the flows the ledger + // hydration left unbound stay unbound here — ⛔ no second check in this + // loop: the gate is shared so no arming path can go without it. for (const name of this.flows.keys()) { if (this.boundFlowTriggers.has(name)) continue; const resolved = this.resolveTriggerBinding(name); @@ -3584,11 +3588,34 @@ export class AutomationEngine implements IAutomationService { /** * Bind a flow to its matching registered trigger (idempotent). No-op when - * the flow has no trigger binding or no trigger is registered for its type - * yet — {@link registerTrigger} re-attempts activation when one arrives. + * the flow may not run ({@link isFlowEnabled}), when it has no trigger + * binding, or when no trigger is registered for its type yet — + * {@link registerTrigger} re-attempts activation when one arrives. */ private activateFlowTrigger(flowName: string): void { if (this.boundFlowTriggers.has(flowName)) return; + // [ADR-0126 §7.2] THE enablement gate for arming — here, at the one + // point every arming path crosses, and not in its callers. A flow + // either disable dimension switches off (the activation ledger, or an + // `obsolete` / `invalid` status) is never handed to a trigger, whichever + // path asks: {@link registerFlow} (boot pull, publish, hot reload), + // {@link registerTrigger}, the enable half of {@link toggleFlow}, and + // any path added later. + // + // Why it cannot live in the callers: `registerTrigger` runs when a + // trigger plugin registers at `kernel:ready`, AFTER `start()` pulled + // the flows and {@link hydrateFlowActivations} unbound the switched-off + // ones. While only `registerFlow` asked, that later registration + // re-armed every one of them on every cold boot — `/_status` reported a + // disabled flow `bound: true`, and each matching event fired a run + // `execute()` then refused. + // + // Silent on purpose: an unarmed disabled flow is the state the switch + // exists to produce, the host's hydration line already named it, and + // `getFlowRuntimeStates()` reports it `enabled: false`. Ahead of the + // scheduled-work policy gate below for the same reason — a flow that + // may not run has no policy refusal to record. + if (!this.isFlowEnabled(flowName)) return; const resolved = this.resolveTriggerBinding(flowName); if (!resolved) return; // [#17396] The deployment gate, read HERE rather than only inside the @@ -3661,18 +3688,44 @@ export class AutomationEngine implements IAutomationService { // landed; the only other trace is the passive run-history row. // That stderr also survives the CLI's boot-quiet stdout window is // stream mechanics, not the verdict. + // + // [ADR-0126 §7.2] The line states only what happened. Two kinds of + // `execute()` answer are not a failed run: + // - `FLOW_DISABLED`: the flow was switched off after the trigger + // took the event — an event in flight at the switch-off, or a + // trigger whose `stop()` failed. The refusal IS the switch + // working, so it is said at `info`, never as an `error` that + // reads as a production failure. The enablement gate above + // keeps a disabled flow from being armed at all, so this is the + // residue, not the steady state. + // - every other never-dispatched exit (a `code` or a missing + // flow, and no `status`): no node ran, so the line claims no + // run-history row. Only a run that dispatched and failed + // carries `status: 'failed'` — the verdict that exit also wrote + // to the run history. trigger.start(resolved.binding, (ctx: AutomationContext) => this.execute(flowName, ctx).then((result) => { - if (!result.success) { - this.logger.error( - `Trigger-fired run of flow '${flowName}' failed (trigger '${resolved.triggerType}') — ` + - `no caller holds this result and nothing retries the run; the terminal failure ` + - `is recorded in the flow's run history, and the run's failure envelope is in ` + - `this record's meta.`, - undefined, - { error: result.error ?? 'unknown error' }, + if (result.success) return; + if (result.code === 'FLOW_DISABLED') { + this.logger.info( + `Trigger '${resolved.triggerType}' fired flow '${flowName}', which is disabled — the ` + + `run was refused before it started, nothing ran, and no run-history row records ` + + `it. The refusal is in this record's meta.`, + { error: result.error }, ); + return; } + this.logger.error( + `Trigger-fired run of flow '${flowName}' failed (trigger '${resolved.triggerType}') — ` + + `no caller holds this result and nothing retries the run; ` + + (result.status === 'failed' + ? `the terminal failure is recorded in the flow's run history, and the run's ` + + `failure envelope is in this record's meta.` + : `it was refused before it dispatched, and the refusal envelope is in this ` + + `record's meta.`), + undefined, + { error: result.error ?? 'unknown error' }, + ); }), ); this.boundFlowTriggers.set(flowName, resolved.triggerType); @@ -4263,14 +4316,15 @@ export class AutomationEngine implements IAutomationService { } // Re-bind in case the definition changed its trigger, then (re)activate. - // [ADR-0126 §7.2] A ledger-disabled flow is NOT re-armed here, which is - // what makes the unbind survive a republish and a restart: the boot - // pull re-registers every flow, so a hydrated ledger row has to be - // able to keep a trigger unbound through exactly this path. + // [ADR-0126 §7.2] A disabled flow — ledger or status — is NOT re-armed + // here, which is what makes the unbind survive a republish and a + // restart: the boot pull re-registers every flow, so a hydrated ledger + // row has to be able to keep a trigger unbound through exactly this + // path. The refusal is `activateFlowTrigger`'s own enablement gate, + // the one every arming path shares — ⛔ not re-asked here, where a + // caller-side check once stood alone and `registerTrigger` had none. this.deactivateFlowTrigger(name); - if (this.isFlowEnabled(name)) { - this.activateFlowTrigger(name); - } + this.activateFlowTrigger(name); // #12206 (Option A) — hand the caller the canonicalized flow this // registration stored: the same object `this.flows` now holds and @@ -4657,6 +4711,9 @@ export class AutomationEngine implements IAutomationService { // A disabled flow should stop receiving trigger events; a re-enabled one // should resume. execute() also guards disabled flows, but unbinding // avoids firing the trigger (and its event-source subscription) at all. + // Re-enabling moves only the LEDGER bit: a flow whose `status` still + // disables it stays unarmed, by `activateFlowTrigger`'s enablement gate + // — armed, it would only fire runs `execute()` refuses. if (enabled) { this.activateFlowTrigger(name); } else {