Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 51 additions & 21 deletions packages/daemon/src/lib/space/workflows/built-in-workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1244,13 +1244,13 @@ export const FULLSTACK_QA_LOOP_WORKFLOW: SpaceWorkflow = {
{
from: 'Review',
to: 'Coding',
maxCycles: 6,
maxCycles: 50,
Comment thread
lsm marked this conversation as resolved.
Comment thread
lsm marked this conversation as resolved.
label: 'Review → Coding (feedback)',
},
{
from: 'QA',
to: 'Coding',
maxCycles: 6,
maxCycles: 50,
label: 'QA → Coding (issues found)',
},
],
Expand Down Expand Up @@ -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<SpaceWorkflow['channels']>[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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update capped cycle records when re-stamping the cap

For an existing Fullstack run that has already reached six traversals—the exact upgrade case this change targets—writing 50 only into the workflow JSON does not unblock it. ChannelCycleRepository.incrementCycleCount guards its conflict update with WHERE count < max_cycles, where the persisted channel_cycles.max_cycles remains 6; canDeliver compares the count against the new workflow limit of 50, but the atomic increment then returns false and delivery still throws. The migration must update existing cycle records, or the increment guard must use the supplied cap, so active runs already at the old limit can continue.

Useful? React with 👍 / 👎.

label: templateChannel.label,
Comment thread
lsm marked this conversation as resolved.
};
});

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(
Expand Down Expand Up @@ -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.
Expand All @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)!;
Expand Down Expand Up @@ -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);
Expand Down
Loading