diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index ec5a96f08..cd344c28b 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -1244,13 +1244,13 @@ export const FULLSTACK_QA_LOOP_WORKFLOW: SpaceWorkflow = { { from: 'Review', to: 'Coding', - maxCycles: 6, + maxCycles: 50, label: 'Review → Coding (feedback)', }, { from: 'QA', to: 'Coding', - maxCycles: 6, + maxCycles: 50, label: 'QA → Coding (issues found)', }, ], @@ -1910,19 +1910,38 @@ function mergeChannelsFromTemplate( ); if (!existingChannels) return remappedTemplateChannels; - const existingKeys = new Set( - existingChannels.map((channel) => - JSON.stringify({ from: channel.from, to: channel.to, gateId: channel.gateId ?? null }) - ) + const channelKey = (channel: NonNullable[number]) => + JSON.stringify({ from: channel.from, to: channel.to, gateId: channel.gateId ?? null }); + + const templateChannelByKey = new Map( + remappedTemplateChannels.map((channel) => [channelKey(channel), channel]) ); + + // In-place merge of structural channel fields (maxCycles, label) onto + // channels that already exist in the seeded workflow, mirroring the gate + // (writers/features) and node-agent (toolGuards) merges. Channels are + // structural topology: {from, to, gateId} is the stable match key and the + // template owns maxCycles + label. Propagating them keeps structural changes + // (e.g. raising a cyclic cap 6 → 50) landing on pre-existing spaces; without + // this, the unconditional templateHash write would stamp the new hash while + // leaving the old field values in place, then block any future fix from + // reaching them (the matching hash skips the row on every later startup). + const mergedExisting = existingChannels.map((channel) => { + const templateChannel = templateChannelByKey.get(channelKey(channel)); + if (!templateChannel) return channel; + return { + ...channel, + maxCycles: templateChannel.maxCycles, + label: templateChannel.label, + }; + }); + + const mergedExistingKeys = new Set(mergedExisting.map(channelKey)); const missingTemplateChannels = remappedTemplateChannels.filter( - (channel) => - !existingKeys.has( - JSON.stringify({ from: channel.from, to: channel.to, gateId: channel.gateId ?? null }) - ) + (channel) => !mergedExistingKeys.has(channelKey(channel)) ); - return [...existingChannels, ...missingTemplateChannels]; + return [...mergedExisting, ...missingTemplateChannels]; } function remapTemplateHookAgentSlots( @@ -2103,10 +2122,16 @@ export function mergeGateStructuralFieldsFromTemplate( * Gate `features` are copied from matching template gates so data-driven runtime * checks land on pre-existing spaces. Missing template gates are appended. * Existing checks, scripts, and gate topology remain untouched. - * - Missing template channels are appended so newly-added built-in branches become - * reachable on pre-existing spaces. Template hooks are copied from the built-in - * template so hook-based runtime metadata lands during drift re-stamps. Existing - * channels, layout, and node rows are not regenerated. Workflow IDs, node IDs, and persisted node-agent slots + * - Structural channel fields (maxCycles, label) are merged in-place onto channels matched by + * {from, to, gateId}, and missing template channels are appended so newly-added built-in + * branches become reachable on pre-existing spaces. This is how a raised cyclic cap (e.g. + * maxCycles 6 → 50) lands on pre-existing spaces instead of only newly-created ones. Like the + * other template-owned structural fields (completionAutonomyLevel, gate writers/features, node + * toolGuards, hooks), built-in channel maxCycles/label are template-managed: a user-customized + * value (editable via the visual editor) is reset to the template value when drift triggers a + * re-stamp — clone to a custom (non-re-stamped) workflow for a persistent custom cap. Template + * hooks are copied from the built-in template so hook-based runtime metadata lands during + * drift re-stamps. Existing channels, layout, and node rows are not regenerated. Workflow IDs, node IDs, and persisted node-agent slots * are stable identifiers for in-flight runs, so template drift must never * replace node rows. Agent `toolGuards` are updated in-place on existing node * configs instead. @@ -2117,7 +2142,7 @@ const RESTAMP_FIELDS = [ 'templateHash', 'nodes(postApproval + toolGuards in-place + missing template nodes)', 'gates(field writers + features in-place + missing template gates)', - 'channels(missing template channels)', + 'channels(maxCycles + label in-place on matched channels + missing template channels)', 'hooks(template hooks)', ] as const; @@ -2206,8 +2231,15 @@ export function seedBuiltInWorkflows( ); const removedLegacyPrReadyChannels = (existingChannels?.length ?? 0) !== (row.channels?.length ?? 0); - const hasNewTemplateChannels = - (mergedChannels?.length ?? 0) > (existingChannels?.length ?? 0); + // mergeChannelsFromTemplate propagates structural fields (maxCycles, + // label) in-place on matched channels in addition to appending any + // missing template channels. Detect whether the merge changed anything + // — added channels OR updated structural fields — so the result is + // persisted even when the channel count is unchanged (e.g. raising a + // cyclic cap 6 → 50 on an already-seeded workflow). + const channelsChanged = + removedLegacyPrReadyChannels || + JSON.stringify(mergedChannels) !== JSON.stringify(existingChannels); workflowManager.updateWorkflow(row.id, { completionAutonomyLevel: template.completionAutonomyLevel, @@ -2219,9 +2251,7 @@ export function seedBuiltInWorkflows( mergeHooksFromTemplate(template.hooks, template.nodes, migratedNodes, row.hooks) ?? null, nodes: migratedNodes, - ...(hasNewTemplateChannels || removedLegacyPrReadyChannels - ? { channels: mergedChannels } - : {}), + ...(channelsChanged ? { channels: mergedChannels } : {}), templateHash: expectedHash, }); restamped.push(template.name); diff --git a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts index 9e70b13f1..89c36d624 100644 --- a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts +++ b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts @@ -1818,6 +1818,54 @@ describe('seedBuiltInWorkflows()', () => { expect(after.templateHash).not.toBe('stale-hash-from-a-prior-pr'); }); + test('re-stamp propagates template maxCycles onto existing Fullstack QA Loop cyclic back-channels', () => { + // Seed fresh — Fullstack QA Loop carries the current template (maxCycles: 50) + // and the current template hash. + seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + const wf = manager + .listWorkflows(SPACE_ID) + .find((w) => w.name === FULLSTACK_QA_LOOP_WORKFLOW.name)!; + + // Simulate a pre-fix seed: both cyclic back-channels carry the old maxCycles: 6. + // This is the state any existing space was left in before the 6 → 50 bump. + manager.updateWorkflow(wf.id, { + channels: (wf.channels ?? []).map((ch) => + (ch.from === 'Review' && ch.to === 'Coding') || (ch.from === 'QA' && ch.to === 'Coding') + ? { ...ch, maxCycles: 6 } + : ch + ), + }); + + // Force the re-stamp path by stamping a stale hash (the persisted hash + // matches the OLD template; the new template's hash differs → drift fires). + db.prepare(`UPDATE space_workflows SET template_hash = ? WHERE id = ?`).run( + 'stale-hash-pre-maxCycles-50', + wf.id + ); + + // Sanity-check the simulated drift landed before re-stamp. + const before = manager.getWorkflow(wf.id)!; + expect(before.channels!.find((c) => c.from === 'Review' && c.to === 'Coding')!.maxCycles).toBe( + 6 + ); + expect(before.channels!.find((c) => c.from === 'QA' && c.to === 'Coding')!.maxCycles).toBe(6); + + // Re-run the seeder — re-stamp branch fires. + const result = seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + expect(result.restamped).toContain(FULLSTACK_QA_LOOP_WORKFLOW.name); + + // Structural channel fields propagated in-place: the template's maxCycles: 50 + // now lands on the already-seeded back-channels. Without the in-place merge + // this silently fails — the old maxCycles: 6 is preserved verbatim while the + // hash is bumped to the 50-hash, permanently blocking future fixes from + // reaching existing spaces (the matching hash skips the row on every restart). + const after = manager.getWorkflow(wf.id)!; + expect(after.channels!.find((c) => c.from === 'Review' && c.to === 'Coding')!.maxCycles).toBe( + 50 + ); + expect(after.channels!.find((c) => c.from === 'QA' && c.to === 'Coding')!.maxCycles).toBe(50); + }); + test('re-stamp does NOT touch handles — custom user handle is preserved', () => { seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); const coding = manager.listWorkflows(SPACE_ID).find((w) => w.name === CODING_WORKFLOW.name)!; @@ -6139,6 +6187,27 @@ test('FULLSTACK_QA_LOOP_WORKFLOW QA node prompt contains Terminal Action Pre-con ); }); +// Regression: PR lsm/HyperNeo#2262 hit the previous maxCycles: 6 cap on +// round 7 of a legitimate review loop, blocking the in-band Review → Coding +// handoff. Both cyclic back-channels must permit well beyond 6 cycles. +test('FULLSTACK_QA_LOOP_WORKFLOW cyclic back-channels permit more than 6 review/QA cycles', () => { + const reviewToCoding = FULLSTACK_QA_LOOP_WORKFLOW.channels!.find( + (c) => c.from === 'Review' && c.to === 'Coding' + ); + const qaToCoding = FULLSTACK_QA_LOOP_WORKFLOW.channels!.find( + (c) => c.from === 'QA' && c.to === 'Coding' + ); + expect(reviewToCoding).toBeDefined(); + expect(qaToCoding).toBeDefined(); + // Explicit cap raised from 6 → 50; pin the exact value so a future + // regression to the old tight cap (or below) is caught immediately. + expect(reviewToCoding!.maxCycles).toBe(50); + expect(qaToCoding!.maxCycles).toBe(50); + // Behavioral guard: both channels must permit more than 6 cycles. + expect(reviewToCoding!.maxCycles).toBeGreaterThan(6); + expect(qaToCoding!.maxCycles).toBeGreaterThan(6); +}); + test('PLAN_AND_DECOMPOSE_WORKFLOW Plan Review reviewers carry Terminal Action Pre-conditions', () => { const reviewNode = PLAN_AND_DECOMPOSE_WORKFLOW.nodes.find((n) => n.name === 'Plan Review')!; expect(reviewNode.agents).toHaveLength(4);