fix: detect completion signal in any XML tag, not just <promise> (#1126)

[Wirasm]

Summary

• Problem: Loop nodes with until: fail when the AI wraps the completion signal in any XML tag other than <promise> (e.g. <COMPLETE>ALL_CLEAN</COMPLETE>), causing max_iterations_reached even though the signal was present
• Why it matters: Any workflow where the prompt instructs the AI to output a tagged signal with a custom tag (a natural pattern) would silently fail the loop and skip all downstream nodes
• What changed: detectCompletionSignal now matches <anyTag>SIGNAL</anyTag> in addition to <promise>SIGNAL</promise> and plain-text patterns; stripCompletionTags accepts an optional until param to strip matched XML tags from user-visible output
• What did not change: The recommended <promise> format, plain-text signal detection, max_iterations_reached error message, and all other executor logic

UX Journey

Before

User workflow prompt: "When done, output <COMPLETE>ALL_CLEAN</COMPLETE>"

  Workflow                dag-executor              detectCompletionSignal
  ────────                ────────────              ──────────────────────
  loop node runs ──────▶  AI outputs                checks <promise>... ✗
  (max_iterations: 3)     <COMPLETE>ALL_CLEAN        checks endPattern... ✗ (</COMPLETE> follows)
                          </COMPLETE>                checks ownLinePattern... ✗
                                                     returns false
                          completionDetected = false
                          (loop exhausts iterations)
  ❌ max_iterations_reached
  downstream nodes skipped

After

User workflow prompt: "When done, output <COMPLETE>ALL_CLEAN</COMPLETE>"

  Workflow                dag-executor              detectCompletionSignal
  ────────                ────────────              ──────────────────────
  loop node runs ──────▶  AI outputs                checks <promise>... ✗
  (max_iterations: 3)     <COMPLETE>ALL_CLEAN        [NEW] checks xmlWrappedPattern... ✓
                          </COMPLETE>                returns true
                          completionDetected = true
                          [strips <COMPLETE>ALL_CLEAN</COMPLETE> from output]
  ✅ loop exits completed
  downstream nodes run

Architecture Diagram

Before

dag-executor.ts
  └── detectCompletionSignal(output, signal)   [executor-shared.ts]
        ├── checks <promise>SIGNAL</promise>
        └── checks plain signal (end / own-line)

  └── stripCompletionTags(content)             [executor-shared.ts]
        └── strips <promise>...</promise>

After

dag-executor.ts
  └── detectCompletionSignal(output, signal)   [executor-shared.ts]
        ├── checks <promise>SIGNAL</promise>
        ├── [NEW] checks <anyTag>SIGNAL</anyTag>   ← additive
        └── checks plain signal (end / own-line)

  └── stripCompletionTags(content, until?)     [executor-shared.ts]
        ├── strips <promise>...</promise>
        └── [NEW] strips <anyTag>SIGNAL</anyTag> when until is provided

Connection inventory:

From	To	Status	Notes
dag-executor.ts	detectCompletionSignal	unchanged	same call, same args
dag-executor.ts	stripCompletionTags	modified	now passes loop.until as second arg
executor-shared.ts	detectCompletionSignal impl	modified	added xmlWrappedPattern branch
executor-shared.ts	stripCompletionTags impl	modified	added optional until param

Label Snapshot

• Risk: risk: low
• Size: size: XS
• Scope: workflows
• Module: workflows:executor

Change Metadata

• Change type: bug
• Primary scope: workflows

Linked Issue

• Closes Loop node reports max_iterations_reached despite completion signal being present in output #1126

Validation Evidence (required)

bun run validate

• Type check: ✅ No errors across all 10 packages
• Lint: ✅ 0 errors, 0 warnings (--max-warnings 0)
• Format: ✅ All files formatted
• Tests: ✅ All tests passed — @archon/workflows: 397 passed (154 in dag-executor, 45 in executor-shared)
• No commands skipped

Security Impact (required)

• New permissions/capabilities? No
• New external network calls? No
• Secrets/tokens handling changed? No
• File system access scope changed? No

The added regex pattern matches only within already-captured AI output strings; no new attack surface.

Compatibility / Migration

• Backward compatible? Yes — the change is purely additive; all existing detection patterns remain unchanged
• Config/env changes? No
• Database migration needed? No

The stripCompletionTags signature change is backward compatible: until is optional with default undefined, so all existing call sites compile and behave identically.

Human Verification (required)

• Verified scenarios:
  • <COMPLETE>ALL_CLEAN</COMPLETE> detected and signal-stripped from output
  • <promise>COMPLETE</promise> format still works
  • Plain bare signal at end of output still works
  • Signal embedded in prose ("not COMPLETE yet") still not detected (false-positive guard)
  • Wrong value in XML tag (<COMPLETE>WRONG</COMPLETE> for signal ALL_CLEAN) not detected
• Edge cases checked: self-closing tags (<COMPLETE/>) do not match (no content)
• What was not verified: live end-to-end with a real Claude session (unit + integration coverage is sufficient for this targeted change)

Side Effects / Blast Radius (required)

• Affected subsystems/workflows: Loop nodes with until: completion signals only
• Potential unintended effects: Low-risk false positive if the AI outputs <code>SIGNAL_VALUE</code> in a code block and SIGNAL_VALUE happens to match the until: signal. This is a pre-existing risk with plain detection as well, and is mitigated by using distinctive signal values.
• Guardrails: Existing test suite covers the false-positive prose case; no new monitoring needed

Rollback Plan (required)

• Fast rollback: revert the two changed lines in executor-shared.ts and one changed line in dag-executor.ts
• Feature flags: none
• Observable failure symptoms: Loop nodes would again report max_iterations_reached when the AI uses non-<promise> XML tags; no data loss or state corruption

Risks and Mitigations

• Risk: Regex false-positive — AI outputs <tag>SIGNAL</tag> in a code example inside its response
  • Mitigation: Signal values should be distinctive (e.g. ALL_CLEAN, DONE). This is consistent with existing guidance and the pre-existing plain-text detection risk.

Summary by CodeRabbit

• Bug Fixes

  • Improved loop workflow execution to correctly handle XML-wrapped completion signals and context-aware completion tag stripping, ensuring proper workflow termination and clean output display.

• Tests

  • Added comprehensive test coverage for completion signal detection and tag stripping across multiple signal formats and edge cases.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b57a8aa3-b154-49b1-9ece-d6d23e1fd7ee

📥 Commits

Reviewing files that changed from the base of the PR and between bf20063 and 07327f3.

📒 Files selected for processing (4)

• packages/workflows/src/dag-executor.test.ts
• packages/workflows/src/dag-executor.ts
• packages/workflows/src/executor-shared.test.ts
• packages/workflows/src/executor-shared.ts

---

📝 Walkthrough

Walkthrough

This PR enhances completion signal detection in loop nodes to recognize XML-wrapped formats (e.g., <COMPLETE>ALL_CLEAN</COMPLETE>) alongside existing formats, and ensures user-visible output has completion tags stripped based on the loop's termination condition. Core changes span test coverage, executor logic, and shared utility functions to fix the issue where loops incorrectly report failure despite successful completion.

Changes

Cohort / File(s)	Summary
Loop Executor packages/workflows/src/dag-executor.ts	Modified executeLoopNode to pass loop.until context to stripCompletionTags, enabling loop-aware tag removal for user-facing output while preserving full output for signal detection.
Completion Signal Utilities packages/workflows/src/executor-shared.ts	Updated detectCompletionSignal to recognize XML-wrapped completion signals with matching tag names (backreference validation) before plain-text detection; expanded stripCompletionTags signature to accept optional until parameter for conditional removal of XML-wrapped completion tags.
Test Suites packages/workflows/src/dag-executor.test.ts, packages/workflows/src/executor-shared.test.ts	Added test case for XML-wrapped loop completion signal; added comprehensive test suites covering detectCompletionSignal and stripCompletionTags across multiple signal formats (<promise>, arbitrary XML wrappers, plain text) and edge cases (mismatched tags, embedded values, tag-less scenarios).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat(web): loop node iteration visibility in workflow execution view #1026: Modifies executeLoopNode in packages/workflows/src/dag-executor.ts, directly impacting the same execution path and loop-completion logic.

Poem

🐰 A loop that circles round and round,
Now spotting signals safe and sound!
<COMPLETE> wrapped in tags so neat,
The rabbit's fix makes logic sweet! ✨

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch archon/task-fix-issue-1126

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Wirasm]

🔍 Comprehensive PR Review

PR: #1184 — fix: detect completion signal in any XML tag, not just <promise>
Reviewed by: 4 specialized agents (code-review, error-handling, test-coverage, comment-quality)
Date: 2026-04-13

---

Summary

Minimal, well-scoped fix (3 source lines changed). The implementation is correct: escapeRegExp prevents regex injection, the optional until parameter is fully backward-compatible, and the main code paths have solid test coverage. All agents voted APPROVE.

Verdict: ✅ APPROVE

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	2
🟢 LOW	7

---

🟡 Medium Issues (Quick Fixes)

1. JSDoc for detectCompletionSignal still says "two formats"

📍 packages/workflows/src/executor-shared.ts:370-379

The JSDoc documents only 2 detection formats; the PR adds a 3rd. A future developer won't know about the XML-wrapping fallback from the doc comment alone.

View fix

/**
 * Detect whether the AI output contains a completion signal.
 *
 * Supports three formats, checked in order:
 * 1. <promise>SIGNAL</promise> - Recommended; prevents false positives in prose
 * 2. <anytag>SIGNAL</anytag> - Any XML-wrapped tag; case-insensitive on tag names
 * 3. Plain SIGNAL - Backwards compatibility; only at end of output or on own line
 *
 * Plain signal detection is restrictive to prevent false positives like "not SIGNAL yet".
 */

---

2. stripCompletionTags not tested when both tag types appear in same chunk

📍 packages/workflows/src/executor-shared.test.ts

The function now has two strip paths (<promise> and XML-tagged). If a future refactor reorders them, tags would silently leak into user output — and no test would catch it.

View fix

it('strips both <promise> and XML-tagged signal when until is provided', () => {
  const input = 'Done. <promise>ALL_CLEAN</promise> <COMPLETE>ALL_CLEAN</COMPLETE>';
  expect(stripCompletionTags(input, 'ALL_CLEAN')).toBe('Done.');
});

---

🟢 Low Issues

View 7 low-priority suggestions

Issue	Location	Suggestion
Superfluous m flag on xmlWrappedPattern — no anchors, no effect	executor-shared.ts:390	Change 'im' → 'i' to match strip path's 'gi'
Tag mismatch: <foo>SIGNAL</bar> triggers detection (intentional but undocumented)	executor-shared.ts:390	Add comment: // Note: opening and closing tag names are not required to match
Per-chunk strip vs full-output detect: split XML tag could produce tag fragments in stream	dag-executor.ts:1597	Pre-existing <promise> behavior; accept and document
Tag-mismatch permissive behavior not pinned in tests	executor-shared.test.ts	Add it('detects signal in mismatched XML tags (permissive)', ...)
DAG integration test doesn't assert user-visible output is clean	dag-executor.test.ts:2930	Assert platform.sendMessage calls contain no <COMPLETE>
xmlWrappedPattern comment doesn't note tag-name independence	executor-shared.ts:390	Append note to existing comment
stripCompletionTags JSDoc doesn't mention until param	executor-shared.ts:403	Append clause or add @param until

---

✅ What's Good

• escapeRegExp applied to user-supplied signal in both detect and strip — correct defense against regex injection from workflow YAML
• until? is fully backward-compatible; all existing call sites unaffected
• Tests cover the most important negative scenarios: wrong value in tags, signal embedded in prose (guarding the primary false-positive risk that motivated the XML format)
• DAG integration test verifies full execution path: completeWorkflowRun called once, failWorkflowRun never called
• Detection and stripping use identical underlying regex — no drift between what is detected and what is removed
• Fix is precisely scoped — no cleanup, no refactoring, no speculative features

---

Reviewed by Archon comprehensive-pr-review workflow

[Wirasm]

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/task-fix-issue-1126
Commit: 9bc5d8f
Philosophy: Fix everything unless clearly a new concern

---

Fixes Applied (7 total)

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	2
🟢 LOW	5

View all fixes

• ✅ JSDoc for detectCompletionSignal documents only two formats (executor-shared.ts:370-379) — Updated to list all three detection formats with descriptions
• ✅ stripCompletionTags not tested for combined tag types (executor-shared.test.ts) — Added test: strips both <promise> and XML-tagged signal in same chunk
• ✅ Superfluous m flag on xmlWrappedPattern (executor-shared.ts:390) — Changed 'im' → 'i' (no anchors, flag was a no-op)
• ✅ Tag mismatch not documented in regex comment (executor-shared.ts:388-390) — Added inline note that tag names are matched independently by design
• ✅ stripCompletionTags JSDoc doesn't mention until (executor-shared.ts:403) — Appended clause describing the until parameter
• ✅ Tag-mismatch permissive behavior not pinned in tests (executor-shared.test.ts) — Added test: detects signal in mismatched XML tags (permissive)
• ✅ DAG integration test doesn't assert output is clean (dag-executor.test.ts:2979) — Added assertions that sendMessage calls contain no raw <COMPLETE> tags

---

Tests Added

• executor-shared.test.ts: detects signal in mismatched XML tags (permissive)
• executor-shared.test.ts: strips both <promise> and XML-tagged signal when until is provided
• dag-executor.test.ts: output-cleanliness assertions on existing XML-wrapped signal test

---

Skipped (1)

Finding	Reason
Per-chunk strip vs full-output detect: split XML tag could leak fragments (dag-executor.ts:1597)	Pre-existing architectural pattern for <promise> stripping; fixing requires non-trivial streaming refactor out of scope for this PR

---

Validation

✅ Type check | ✅ Lint | ✅ Tests (47 passed executor-shared, 154 passed dag-executor)

---

Self-fix by Archon · aggressive mode · fixes pushed to archon/task-fix-issue-1126