fix(workflows): add word boundary to context variable substitution regex

[coleam00]

Summary

• Problem: substituteWorkflowVariables() uses CONTEXT_VAR_PATTERN_STR with no word boundary or negative lookahead, causing $CONTEXT_FILE to match as $CONTEXT + _FILE, silently corrupting the variable to _FILE at runtime.
• Why it matters: Bash nodes using any shell variable whose name starts with CONTEXT, EXTERNAL_CONTEXT, or ISSUE_CONTEXT (e.g. $CONTEXT_FILE, $ISSUE_CONTEXT_ID) receive a mangled script — no error is thrown, the bash command runs with wrong variable names, and fallback defaults may hide the corruption entirely.
• What changed: Added (?![A-Za-z0-9_]) negative lookahead to CONTEXT_VAR_PATTERN_STR in executor-shared.ts; added a regression test in executor-shared.test.ts.
• What did not change: All other variable substitutions ($WORKFLOW_ID, $ARTIFACTS_DIR, etc.) are unaffected; no changes to context fetching, storage, or any other module.

UX Journey

Before

User workflow YAML (bash node):
  CONTEXT_FILE=".planning/phases/$PHASE/CONTEXT.md"

substituteWorkflowVariables():
  Pattern: \$(?:CONTEXT|EXTERNAL_CONTEXT|ISSUE_CONTEXT)
  Match:   $CONTEXT  (prefix inside $CONTEXT_FILE)
  Result:  CONTEXT_FILE → _FILE  ← silent corruption

bash node receives: _FILE=".planning/phases/$PHASE/CONTEXT.md"
bash: grep: _FILE: No such file or directory
  (or silently produces wrong output if $CONTEXT had a value)

After

User workflow YAML (bash node):
  CONTEXT_FILE=".planning/phases/$PHASE/CONTEXT.md"

substituteWorkflowVariables():
  Pattern: \$(?:CONTEXT|EXTERNAL_CONTEXT|ISSUE_CONTEXT)(?![A-Za-z0-9_])
  No match on $CONTEXT_FILE (followed by '_')
  $CONTEXT alone still matches correctly

bash node receives: CONTEXT_FILE=".planning/phases/$PHASE/CONTEXT.md"
  (correct — user-defined variable preserved)

Architecture Diagram

Before

executor-shared.ts
  CONTEXT_VAR_PATTERN_STR = \$(?:CONTEXT|EXTERNAL_CONTEXT|ISSUE_CONTEXT)
       |
       ▼
  substituteWorkflowVariables()  ← matches $CONTEXT_FILE as $CONTEXT + _FILE
       |
       ▼
  dag-executor.ts / executor.ts  ← receives corrupted prompt
       |
       ▼
  bash node subprocess  ← runs with mangled variable names

After

executor-shared.ts
  CONTEXT_VAR_PATTERN_STR = \$(?:CONTEXT|EXTERNAL_CONTEXT|ISSUE_CONTEXT)(?![A-Za-z0-9_])
       |
       ▼
  substituteWorkflowVariables() [~]  ← only matches standalone context vars
       |
       ▼
  dag-executor.ts / executor.ts  ← receives correct prompt (unchanged)
       |
       ▼
  bash node subprocess  ← runs with correct variable names

Connection inventory:

From	To	Status	Notes
CONTEXT_VAR_PATTERN_STR	substituteWorkflowVariables()	modified	Negative lookahead added to pattern
substituteWorkflowVariables()	buildPromptWithContext()	unchanged	Picks up fix automatically
buildPromptWithContext()	dag-executor.ts / executor.ts	unchanged

Label Snapshot

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

Change Metadata

• Change type: bug
• Primary scope: workflows

Linked Issue

• Closes Variable substitution matches $CONTEXT as prefix of $CONTEXT_FILE — no word boundary #1112

Validation Evidence (required)

bun run validate
# type-check: ✅ pass (all 10 packages)
# lint:       ✅ 0 errors, 0 warnings
# format:     ✅ all files formatted
# tests:      ✅ all packages passed, 0 failures

• Evidence provided: All four bun run validate checks passed on first run; new regression test added and passing.
• 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

Compatibility / Migration

• Backward compatible? Yes — the fix only prevents incorrect matches; all previously correct substitutions continue to work.
• Config/env changes? No
• Database migration needed? No

Human Verification (required)

• Verified scenarios: Regression test covers $CONTEXT (substituted) vs $CONTEXT_FILE (preserved) vs $EXTERNAL_CONTEXT_PATH (preserved) in a single call.
• Edge cases checked: $CONTEXT at end of string (no trailing char), followed by whitespace/newline/quote/colon — all still match. $ISSUE_CONTEXT_ID — now correctly left untouched.
• What was not verified: Live bash node execution with a real workflow run (validated via unit test instead).

Side Effects / Blast Radius (required)

• Affected subsystems: substituteWorkflowVariables() in @archon/workflows — used by every prompt node (bash, command, prompt, loop, approval, script). Impact is purely corrective.
• Potential unintended effects: None — the lookahead is zero-width and only restricts matches where the pattern was already wrong.
• Guardrails: Regression test locks in correct behavior.

Rollback Plan (required)

• Fast rollback: Revert commit 2b0adc24 (git revert 2b0adc24).
• Feature flags: None.
• Observable failure symptoms: Bash nodes using $CONTEXT_FILE or similar begin producing _FILE-style mangled variable names; no error thrown.

Risks and Mitigations

• Risk: None. Change is a one-character regex tightening with a dedicated regression test. No architectural changes.

Summary by CodeRabbit

• Bug Fixes

  • Improved workflow variable substitution accuracy to prevent incorrect matching when context variable names appear as prefixes within longer identifiers (e.g., $CONTEXT_FILE).

• Tests

  • Added test coverage for edge cases around context variable name boundary conditions.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2e7aba19-77c9-4191-ae71-29ea7b960120

📥 Commits

Reviewing files that changed from the base of the PR and between 7721259 and 148b474.

📒 Files selected for processing (2)

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

---

📝 Walkthrough

Walkthrough

This PR fixes a bug in context variable substitution where patterns like $CONTEXT were incorrectly matched as prefixes of longer identifiers (e.g., $CONTEXT_FILE). A negative lookahead assertion was added to the regex pattern, and comprehensive test cases were added to verify the fix.

Changes

Cohort / File(s)	Summary
Pattern Fix packages/workflows/src/executor-shared.ts	Updated CONTEXT_VAR_PATTERN_STR to include negative lookahead (?![A-Za-z0-9_]), preventing $CONTEXT from matching when followed by identifier characters.
Test Coverage packages/workflows/src/executor-shared.test.ts	Added three test cases verifying that context variables are not incorrectly substituted when they appear as prefixes (e.g., $CONTEXT_FILE, $ISSUE_CONTEXT_ID) or when contextSubstituted should remain false.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A boundary was lost in the regex's sight,
$CONTEXT nibbled $CONTEXT_FILE in the night,
Now lookahead guards the alphanumeric way,
Variables stay whole—hooray, hooray! 🎉

✨ 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-1112-variable-boundary

---

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.}

[coleam00]

🔍 Comprehensive PR Review

PR: #1256
Reviewed by: 5 specialized agents (code-review, error-handling, test-coverage, comment-quality, docs-impact)
Date: 2026-04-16

---

Summary

Minimal, targeted fix that adds a negative lookahead (?![A-Za-z0-9_]) to CONTEXT_VAR_PATTERN_STR, preventing context variables ($CONTEXT, $EXTERNAL_CONTEXT, $ISSUE_CONTEXT) from matching as prefixes of longer user-defined variable names like $CONTEXT_FILE. The fix is correct, applies symmetrically to both the detection and replacement paths (single shared constant), and is backed by a direct regression test for issue #1112.

Verdict: ✅ APPROVE

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	0
🟢 LOW	3

---

🟢 Low Issues (Optional Test Additions)

All three are test documentation improvements — none represent functional gaps.

1. $ISSUE_CONTEXT_ID suffix case not explicitly tested

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

The new regression test covers $CONTEXT_FILE and $EXTERNAL_CONTEXT_PATH but not $ISSUE_CONTEXT_ID. The regex is correct for all three alternations, but explicit coverage documents the intent.

One-liner fix (extend existing test)

it('does not treat context variables as prefixes of longer identifiers', () => {
  const { prompt, contextSubstituted } = substituteWorkflowVariables(
    'Context: $CONTEXT. File: $CONTEXT_FILE. External: $EXTERNAL_CONTEXT_PATH. IssueId: $ISSUE_CONTEXT_ID',
    'run-1', 'msg', '/tmp', 'main', 'docs/', 'context-data'
  );
  expect(prompt).toBe(
    'Context: context-data. File: $CONTEXT_FILE. External: $EXTERNAL_CONTEXT_PATH. IssueId: $ISSUE_CONTEXT_ID'
  );
  expect(contextSubstituted).toBe(true);
});

---

2. hasContextVariables detection not tested for suffix-only prompts

📍 packages/workflows/src/executor-shared.ts:299

When a prompt contains only $CONTEXT_FILE (no bare $CONTEXT), contextSubstituted should be false — ensuring buildPromptWithContext still appends context. No test asserts this for the detection path.

Suggested test

it('does not set contextSubstituted when only suffix-extended context vars are present', () => {
  const { prompt, contextSubstituted } = substituteWorkflowVariables(
    'Path: $CONTEXT_FILE',
    'run-1', 'msg', '/tmp', 'main', 'docs/', 'context-data'
  );
  expect(prompt).toBe('Path: $CONTEXT_FILE');
  expect(contextSubstituted).toBe(false);
});

---

3. buildPromptWithContext end-to-end with fixed boundary

📍 packages/workflows/src/executor-shared.test.ts (lines 227-272)

No buildPromptWithContext test exercises the full-stack fix (prompt with $CONTEXT_FILE only → context should still be appended). Low risk since detection and replacement share a single constant, making desync structurally impossible.

Recommendation: Skip — the shared constant makes a desync bug impossible without a concurrent structural change.

---

✅ What's Good

• Minimal blast radius: 2-line source change + 16-line test, zero collateral modifications
• Atomic fix: CONTEXT_VAR_PATTERN_STR drives both hasContextVariables (line 299) and replace (line 311) — fix propagates to both paths automatically
• Correct mechanism: (?![A-Za-z0-9_]) is the right tool — more explicit than \b for shell variable name suffix characters
• Fresh regex instances: new RegExp(CONTEXT_VAR_PATTERN_STR) at call time avoids lastIndex reuse bugs
• Test covers the real trigger: $CONTEXT_FILE and $EXTERNAL_CONTEXT_PATH are exactly the user-defined variables from Variable substitution matches $CONTEXT as prefix of $CONTEXT_FILE — no word boundary #1112 that were silently corrupted
• Test isolation maintained: executor-shared.test.ts runs in its own bun test invocation — no mock pollution
• All CLAUDE.md rules pass: Fail Fast, no any types, KISS, YAGNI, SRP all confirmed

---

Reviewed by Archon comprehensive-pr-review workflow
Artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/d3e5a8d81144294a66cb0f5d090e995a/review/

[coleam00]

Review Fix Report

All review findings addressed in commit 148b4741.

Changes Made

Finding 1 (code-review + test-coverage): Extended the existing boundary test to include $ISSUE_CONTEXT_ID in the input string — all three alternation branches (CONTEXT, EXTERNAL_CONTEXT, ISSUE_CONTEXT) are now explicitly verified.

Finding 2 (test-coverage): Added new test 'does not substitute $ISSUE_CONTEXT when followed by identifier characters' covering both $ISSUE_CONTEXT_ID and $ISSUE_CONTEXT_TYPE to document that all suffix variants of the third alternation are guarded.

Finding 3 (test-coverage): Added new test 'does not set contextSubstituted when only suffix-extended context vars are present' asserting contextSubstituted: false when a prompt contains only $CONTEXT_FILE — closes the detection-path regression story.

Finding 4 (test-coverage): Deferred per reviewer recommendation — buildPromptWithContext test for $CONTEXT_FILE-only prompt skipped since detection and replacement share a single constant, making a desync structurally impossible.

Validation

• bun run type-check: All 10 packages passed
• bun run lint: Passed (0 warnings)
• bun test packages/workflows/src/executor-shared.test.ts: 38 pass, 0 fail

[coleam00]

Archon PR Validation Report

Verdict: APPROVE

Summary

Clean, minimal, correct fix. The negative lookahead (?![A-Za-z0-9_]) on CONTEXT_VAR_PATTERN_STR prevents $CONTEXT_FILE from being silently corrupted to _FILE. Three regression tests lock in the fix. No side effects, no regressions.

Bug Confirmation

Claim	Main	Feature
CONTEXT_VAR_PATTERN_STR lacks word boundary	Confirmed	Fixed
Silent corruption of bash variables ($CONTEXT_FILE → _FILE)	Confirmed	Fixed
Detection path (hasContextVariables) falsely triggered	Confirmed	Fixed
No test coverage for prefix-match behavior	Confirmed	3 regression tests added

Fix Quality: 5/5

Issues

No blocking issues found.

Minor Suggestion

Other variable substitutions (e.g., $ARTIFACTS_DIR) could theoretically have the same prefix-matching issue — appropriately deferred per YAGNI.

---

Validated by archon-validate-pr workflow

[coleam00]

Credit to @txhno, who independently reported and fixed this first in #1166 before this PR was opened. Thank you for the catch and the clean fix — this PR carries the same one-line regex change plus additional regression tests (covering $ISSUE_CONTEXT_* and the contextSubstituted detection path), so we're merging this one and closing #1166 as superseded.