fix: raise Fullstack QA Loop cyclic channel maxCycles 6 → 50

[lsm]

What

Raises the cyclic-channel cycle cap on the Fullstack QA Loop built-in workflow from 6 → 50 on both cyclic back-channels, and fixes the re-stamp path so the bump reaches existing spaces (not just newly-created ones):

• Template: Review → Coding + QA → Coding maxCycles: 6 → 50 (built-in-workflows.ts)
• Re-stamp: mergeChannelsFromTemplate now merges maxCycles + label in-place on channels matched by {from,to,gateId}; seedBuiltInWorkflows now persists channels whenever the merge changes anything, not only on add/remove.

Why

PR #2262 hit the previous maxCycles: 6 cap on round 7 of a legitimate review loop — the Reviewer's in-band send_message to Coding was rejected with "Cyclic channel ... has reached the maximum cycle count (6)." 7 rounds is legitimate for a non-trivial PR; a cap of 6 is too tight.

The propagation bug (review feedback)

The template change alone did not reach existing spaces. mergeChannelsFromTemplate matched channels by {from,to,gateId} and returned existing channels verbatim, so the back-channels kept maxCycles: 6. Meanwhile the re-stamp wrote templateHash unconditionally but wrote channels only on add/remove — so on the first restart the hash was bumped to the 50-hash while channels stayed at 6, after which the matching hash skipped the row on every future startup. Net effect: the increase reached only newly-created spaces; every existing space (incl. the #2262 trigger) kept maxCycles:6 permanently with no self-heal path.

Fix mirrors the existing gate (writers/features) and node-agent (toolGuards) in-place merges — the template owns structural channel fields, so changes propagate to pre-existing spaces.

Tests

• Regression test pinning both Fullstack back-channels to maxCycles: 50 (+ > 6 guard).
• Re-stamp regression: seed Fullstack → regress both back-channels to 6 → stamp a stale hash → re-run seeder → assert persisted maxCycles === 50. Verified to fail (Received: 6) without the propagation fix.

Scope

• Runtime fallback ?? 5 (channel-router.ts, space-runtime.ts) untouched — separate concern from the explicit template value.
• Other built-in workflows' explicit maxCycles: 5 left as-is.

Verification

• 0-shared-handlers-workflow (built-in-workflows, template-hash, migration-replay) — 3223 pass / 0 fail
• 4-space-storage (channel persistence, migrations) — 1830 pass / 0 fail
• 5-space-agent-other (channel-router) — 1503 pass / 0 fail
• 5-space-runtime-a — 390 pass / 0 fail
• bun run check (lint + typecheck + knip + session-guards + db-schema-parity + test-quality) — green

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e9983a09a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[lsm]

🤖 Review by glm-5.1[1m] (NeoKai)

Model: glm-5.1[1m] | Client: NeoKai | Provider: z.ai

Recommendation: REQUEST_CHANGES — 1× P1 (commenting, not blocking-via-event, because this PR is authored by the same GitHub account)

What's correct

The template change is right and minimal: both Fullstack back-channels raised 6 → 50 (built-in-workflows.ts:1247, 1253). The runtime honors an explicit maxCycles (channel-router.ts:603 — channel.maxCycles ?? 5 only falls back when undefined; isCycleCapReached is count >= maxCycles). The regression test is well-targeted (toBe(50) + > 6 guard) and passes; no existing test asserted the old 6. Scope is otherwise tight — the runtime ?? 5 fallback and other workflows' 5 are correctly untouched. Verified green locally: built-in-workflows.test.ts (298 pass / 0 fail), channel-router + template-hash + workflow-migration-replay (166 pass / 0 fail).

P1 — the cap bump never reaches existing spaces (re-stamp stamps the hash without the value)

See the anchored comment on built-in-workflows.ts:1247 (the unresolved Codex thread raises the same; I independently verified the mechanics). mergeChannelsFromTemplate (1901-1926) preserves matched channels verbatim, while the re-stamp writes the new templateHash unconditionally (2225) but writes channels only on add/remove (2222-2224). Result: every pre-existing space — including the one that hit #2262 — stays at maxCycles: 6 for all future runs, and the now-matching hash means it is never reconsidered. The PR body's "new runs only" framing understates this: it is "newly-created spaces only", and the hash-stamp blocks future self-heal. The fix belongs in this PR (propagate maxCycles/label for matched channels in mergeChannelsFromTemplate, mirroring the in-place gate/node merges in RESTAMP_FIELDS); shipping as-is permanently silences the drift on existing spaces.

Minor (non-blocking)

docs/research/pr-merging-completion-actions.md:86,124,125 still shows maxCycles=6 — but it's a dated (2026-04-21) point-in-time research doc already stale on other axes, so leaving it as a historical snapshot is defensible.

Summary

The template change is good to approve; please address the P1 propagation gap (plus a re-stamp test asserting maxCycles lands on a pre-seeded workflow) before merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c6a1f57e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[lsm]

🤖 Review by glm-5.1[1m] (NeoKai)

Model: glm-5.1[1m] | Client: NeoKai | Provider: z.ai

Recommendation: REQUEST_CHANGES — the original P1 is fully resolved and the code is correct, but one review conversation is still unresolved and must be consciously addressed before merge. (Commenting rather than blocking-via-event because this PR is authored by the same GitHub account.)

Original P1 (cap not propagating to existing spaces) — RESOLVED ✓

Verified: mergeChannelsFromTemplate (built-in-workflows.ts:1913-1942) now copies maxCycles + label from the template onto channels matched by {from, to, gateId}, and seedBuiltInWorkflows persists channels whenever the merge changes anything (channelsChanged = removedLegacyPrReadyChannels || JSON.stringify(merged) !== JSON.stringify(existing), lines 2236-2238) — not only on add/remove. The new re-stamp regression test (seed → regress back-channels to 6 → stale hash → re-seed → assert 50) covers exactly the path that previously failed silently. Green locally: built-in-workflows + channel-router + template-hash + migration-replay + channel-cycle-repository (474 pass / 0 fail) and 5-space-runtime-a (390 pass / 0 fail). The hash-stamp poison pill is closed.

Open thread — overwrite of user-customized maxCycles/label (Codex, line 1935, unresolved)

I hit the same concern independently before reading the Codex thread: maxCycles is user-editable (ChannelEdgeConfigPanel exposes it; spaceWorkflow.update passes channel edits through with no built-in guard), yet the new merge overwrites it unconditionally on any drift. So a deliberately-lowered cap on a built-in channel can be silently reset to 50 by an unrelated template change (prompt/hook/gate edit). Pre-PR, existing channel maxCycles was never touched by re-stamp — this is a real behavior change.

My severity: P2, not P1. The "template owns structural fields" model is already pervasive and unconditional in this same updateWorkflow call — completionAutonomyLevel (line 2241), gate writers/features, node toolGuards, hooks, and postApproval are all overwritten on drift. Channel maxCycles/label joining that list is consistent with the established design; built-in workflows are template-managed by intent (a user wanting a persistent custom cap can clone to a custom workflow, which isn't re-stamped). The robust alternative — preserving user-diverged values — needs per-channel "customized" tracking or prior-template state: real complexity for a narrow edge case, which the repo's simplicity-first guidance weighs against.

What I need to approve: resolve the open Codex thread with a conscious decision — either

• (a, my recommendation) accept + document the template-managed model: add a one-line note to the RESTAMP_FIELDS JSDoc that built-in channel maxCycles/label are template-managed and reset on drift (so the behavior isn't silently surprising), then reply-to/resolve the thread with that rationale; or
• (b) implement user-divergence preservation if you decide maxCycles warrants special treatment.

Either is fine. I can't approve while the conversation is unresolved — a reviewer thread flagged P1 by an independent reviewer shouldn't be silently dropped.

Summary

The propagation fix is correct and well-tested — nice work closing the hash-stamp gap. The sole merge blocker is the unresolved conversation on overwrite semantics; resolve it (I recommend accept + a one-line doc note) and I'll approve next round.

[lsm]

🤖 Review by glm-5.1[1m] (NeoKai)

Model: glm-5.1[1m] | Client: NeoKai | Provider: z.ai

Recommendation: APPROVE — zero findings; all 3 review conversations resolved. (Commenting rather than an APPROVE event because this PR is authored by the same GitHub account — the recommendation is explicit here.)

Verified this round (fresh, from scratch)

• Original P1 — cap now propagates to existing spaces. mergeChannelsFromTemplate copies maxCycles/label in-place onto channels matched by {from,to,gateId}; seedBuiltInWorkflows persists channels whenever the merge changes anything (channelsChanged, lines 2236-2238); the re-stamp regression test pins seed→6→stale-hash→re-seed→50. The hash-stamp poison pill (new hash written while the old value lingered, then the row skipped forever) is closed. Green: built-in-workflows + channel-router + template-hash + migration-replay + channel-cycle-repository (474 pass / 0 fail), 5-space-runtime-a (390 pass / 0 fail).
• Overwrite of user-customized maxCycles/label — consciously decided + documented. The unconditional overwrite is consistent with the existing template-managed model: completionAutonomyLevel (line 2241), gate writers/features, node toolGuards, hooks, and postApproval are all overwritten on drift in the same updateWorkflow call. Commit 5cb7b48a adds a clear RESTAMP_FIELDS JSDoc note that built-in channel caps/labels are template-managed and reset on drift, with the clone-to-custom escape hatch. The rejected existing===6 heuristic was correctly dismissed (hardcodes a historical default, doesn't generalize to future bumps like 50→100, conflates "default" with "untouched"). Matches the repo's simplicity-first guidance.
• Conversations: all 3 review threads resolved (GraphQL: 0 unresolved). PR is OPEN and MERGEABLE.
• This round's diff (5cb7b48a) is doc-only — no code-logic change, so the round-2 green test state carries forward.

Summary

The 6→50 cap now lands on existing spaces (not just newly-created ones), the hash-stamp gap is closed, the overwrite semantics are consciously accepted and documented, and all review threads are resolved. Approving.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5cb7b48a40

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

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 👍 / 👎.