diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index f696269..5d39738 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -70,7 +70,9 @@ program .description('Start the OpenHop API server (:8787) and web UI (:8788)') .option('-p, --port ', 'API port', '8787') .option('--no-web', 'Start API only, skip the web UI dev server') - .action((opts) => { + .option('--no-wait-ready', "Don't probe /health and don't print the ready line on stdout") + .option('--ready-timeout ', 'How long to wait for readiness before giving up', '60') + .action(async (opts) => { const cliDir = resolve(import.meta.dirname, '..', '..') const serverEntry = resolve(cliDir, 'server', 'src', 'index.ts') const webDir = resolve(cliDir, 'web') @@ -119,6 +121,37 @@ program web?.kill('SIGTERM') process.exit(code ?? ExitCode.SUCCESS) }) + + // Readiness probe. Default-on: poll /health until it returns 200, then + // emit a stable, machine-parseable line on stdout. This is the only + // thing `serve` puts on stdout, so callers can do: + // openhop serve & wait_for=$(grep -m1 '^openhop: ready ' fd 1) + // Without --no-wait-ready, downstream scripts have to poll /health + // themselves to know when they can push. + if (opts.waitReady !== false) { + const timeoutSec = Number.parseInt(opts.readyTimeout, 10) || 60 + const apiUrl = `http://localhost:${opts.port}` + const webPart = opts.web !== false ? ` web=http://localhost:8788` : '' + const t0 = Date.now() + const deadline = t0 + timeoutSec * 1000 + while (Date.now() < deadline) { + try { + const r = await fetch(`${apiUrl}/health`) + if (r.ok) { + const elapsed = Math.round((Date.now() - t0) / 100) / 10 + process.stdout.write(`openhop: ready api=${apiUrl}${webPart} elapsed=${elapsed}s\n`) + break + } + } catch { + // Not ready yet — back off and retry. + } + await new Promise((r) => setTimeout(r, 500)) + } + if (Date.now() >= deadline) { + errStderr(red(`✗ API did not become ready within ${timeoutSec}s. Check logs above.`)) + // Don't exit — the children may still come up. Just warn. + } + } }) // --- push --- diff --git a/packages/shared/__tests__/patch.test.ts b/packages/shared/__tests__/patch.test.ts index 797e05d..f047992 100644 --- a/packages/shared/__tests__/patch.test.ts +++ b/packages/shared/__tests__/patch.test.ts @@ -216,13 +216,13 @@ describe('applyPatch', () => { }) describe('add-steps', () => { - it('inserts a step at the beginning (after=-1)', () => { + it('inserts at the beginning (index: 0)', () => { const root = makeRoot() const result = applyPatch(root, { operations: [ { op: 'add-steps', - after: -1, + index: 0, steps: [{ from: 'b', to: 'a', data: 'response' }], }, ], @@ -236,13 +236,15 @@ describe('applyPatch', () => { }) }) - it('inserts a step after a given index', () => { + it('inserts at a middle index (existing step gets pushed right)', () => { + // makeRoot has 1 step (a→b). Insert at index 1 → becomes the 2nd step, + // existing step stays at index 0. const root = makeRoot() const result = applyPatch(root, { operations: [ { op: 'add-steps', - after: 0, + index: 1, steps: [{ from: 'b', to: 'a', data: 'response' }], }, ], @@ -307,7 +309,7 @@ describe('applyPatch', () => { { op: 'remove-steps', indices: [0] }, { op: 'add-steps', - after: -1, + index: 0, steps: [ { from: 'a', to: 'c', data: 'new step' }, { from: 'c', to: 'b', data: 'another' }, @@ -330,7 +332,7 @@ describe('applyPatch', () => { operations: [ { op: 'add-steps', - after: 0, + index: 1, steps: [{ from: 'a', to: 'ghost', data: 'bad' }], }, ], @@ -351,18 +353,60 @@ describe('applyPatch', () => { describe('edge cases', () => { it('add-steps reports out-of-range insertion position', () => { - const root = makeRoot() + const root = makeRoot() // 1 step → valid range is 0..1 const result = applyPatch(root, { operations: [ { op: 'add-steps', - after: 99, + index: 99, steps: [{ from: 'a', to: 'b', data: 'x' }], }, ], }) expect(result.success).toBe(false) expect(result.errors[0].message).toContain('out of range') + // Error should teach the recovery: the valid range and the + // "omit to append" shortcut. + expect(result.errors[0].message).toContain('omit to append') + }) + + it('add-steps appends when index is omitted (no need to know steps.length)', () => { + const root = makeRoot() + const initialCount = root.flow.steps?.length ?? 0 + const result = applyPatch(root, { + operations: [ + { + op: 'add-steps', + steps: [ + { from: 'a', to: 'b', data: 'appended-1' }, + { from: 'b', to: 'a', data: 'appended-2' }, + ], + }, + ], + }) + expect(result.success).toBe(true) + const steps = result.data!.flow.steps! + expect(steps).toHaveLength(initialCount + 2) + // The two new steps must be at the very end, in order. + expect(steps[steps.length - 2]).toMatchObject({ data: 'appended-1' }) + expect(steps[steps.length - 1]).toMatchObject({ data: 'appended-2' }) + }) + + it('add-steps with explicit index === steps.length also appends', () => { + const root = makeRoot() + const initialCount = root.flow.steps?.length ?? 0 + const result = applyPatch(root, { + operations: [ + { + op: 'add-steps', + index: initialCount, + steps: [{ from: 'a', to: 'b', data: 'appended' }], + }, + ], + }) + expect(result.success).toBe(true) + const steps = result.data!.flow.steps! + expect(steps[steps.length - 1]).toMatchObject({ data: 'appended' }) }) it('add-steps initializes the steps array when absent', () => { @@ -379,7 +423,7 @@ describe('applyPatch', () => { operations: [ { op: 'add-steps', - after: -1, + // Omit index → append to a freshly-created empty array. steps: [{ from: 'a', to: 'b', data: 'first' }], }, ], @@ -467,7 +511,7 @@ describe('applyPatch', () => { // Now adds a step referencing the removed 'b' { op: 'add-steps', - after: -1, + index: 0, steps: [{ from: 'a', to: 'b', data: 'orphan' }], }, ], diff --git a/packages/shared/src/patch.ts b/packages/shared/src/patch.ts index fd940fa..1cd7b46 100644 --- a/packages/shared/src/patch.ts +++ b/packages/shared/src/patch.ts @@ -63,7 +63,12 @@ const clearFlowsOp = z.object({ const addStepsOp = z.object({ op: z.literal('add-steps'), - after: z.number().int().min(-1), // insert position + // 0-based position the new steps will occupy after insertion. + // Valid range [0, steps.length] inclusive. Same semantics as + // Array.prototype.splice: 0 prepends, steps.length appends. + // Omit to append — the most common case, and it doesn't require + // knowing the current steps.length. + index: z.number().int().min(0).optional(), steps: z.array(z.any()).min(1), }) @@ -211,11 +216,11 @@ export function applyPatch(root: Root, patch: PatchOperations): PatchResult { case 'add-steps': { if (!result.flow.steps) result.flow.steps = [] - const insertIndex = op.after + 1 + const insertIndex = op.index ?? result.flow.steps.length if (insertIndex > result.flow.steps.length) { errors.push({ path: 'flow.steps', - message: `Insert position ${op.after} is out of range`, + message: `index ${op.index} is out of range (valid: 0..${result.flow.steps.length}; omit to append)`, }) } else { result.flow.steps.splice(insertIndex, 0, ...op.steps) diff --git a/skills/openhop/SKILL.md b/skills/openhop/SKILL.md index a6e851e..17d6654 100644 --- a/skills/openhop/SKILL.md +++ b/skills/openhop/SKILL.md @@ -320,17 +320,17 @@ data: All operations support multiple items. Apply with `openhop patch `. -| Operation | Fields | Description | -| ------------ | ------------------------------------------ | ----------------------------- | -| add-nodes | nodes: [{id, label, type?, icon?, color?}] | Add nodes | -| remove-nodes | nodes: ["id1", "id2"] | Remove nodes + their steps | -| rename-nodes | nodes: [{id, label}] | Change labels | -| update-nodes | nodes: [{id, type?, icon?, color?}] | Update properties | -| set-flows | nodes: [{id, flow: {nodes, steps}}] | Add sub-flows | -| clear-flows | nodes: ["id1"] | Remove sub-flows | -| add-steps | after: N, steps: [...] | Insert steps (-1 = beginning) | -| remove-steps | indices: [0, 3] | Remove steps by index | -| update-step | index: N, step: {...} | Replace a step | +| Operation | Fields | Description | +| ------------ | ------------------------------------------ | ------------------------------------------------------------------------------------------- | +| add-nodes | nodes: [{id, label, type?, icon?, color?}] | Add nodes | +| remove-nodes | nodes: ["id1", "id2"] | Remove nodes + their steps | +| rename-nodes | nodes: [{id, label}] | Change labels | +| update-nodes | nodes: [{id, type?, icon?, color?}] | Update properties | +| set-flows | nodes: [{id, flow: {nodes, steps}}] | Add sub-flows | +| clear-flows | nodes: ["id1"] | Remove sub-flows | +| add-steps | index?: N, steps: [...] | Insert steps at 0-based `index` (same semantics as `Array.splice`). Omit `index` to append. | +| remove-steps | indices: [0, 3] | Remove steps by index | +| update-step | index: N, step: {...} | Replace a step | ## Icons