Skip to content

Fix workflow bash output quoting#1953

Open
cnYui wants to merge 1 commit into
coleam00:devfrom
cnYui:codex/fix-workflow-bash-output-quoting
Open

Fix workflow bash output quoting#1953
cnYui wants to merge 1 commit into
coleam00:devfrom
cnYui:codex/fix-workflow-bash-output-quoting

Conversation

@cnYui

@cnYui cnYui commented Jun 11, 2026

Copy link
Copy Markdown

Summary

  • Problem: bash nodes in checked-in workflows still double-quoted $node.output substitutions even though Archon injects those values pre-quoted.
  • Why it matters: shipped defaults and smoke workflows kept triggering the validator warning and modeled a bash footgun for workflow authors.
  • What changed: rewrote the reported bash bodies to assign $node.output substitutions unquoted first, then quote normal shell variables where needed; regenerated bundled defaults for archon-fix-github-issue.
  • What did not change (scope boundary): No validator/runtime logic changes, no when:/prompt:/command: text changes, and the intentional negative smoke in e2e-structured-output-failfast is left as-is.

UX Journey

Before

Workflow author/user      Archon validator              Bundled workflows
────                      ───────────────              ─────────────────
runs validate ─────────▶  scans bash nodes
                          sees "$node.output" ───────▶ shipped default + smoke workflows
                          emits [bash] warnings
sees noisy output ◀────── cannot distinguish real cleanup from expected negative smoke

After

Workflow author/user      Archon validator              Bundled workflows
────                      ───────────────              ─────────────────
runs validate ─────────▶  scans bash nodes
                          [tracked workflow bash refs use unquoted assignment]
                          [only intentional negative smoke warns]
cleaner signal ◀────────  shipped default no longer models the footgun

Architecture Diagram

Before

.archon/workflows/defaults/archon-fix-github-issue.yaml
  └─ fetch-issue bash node: ISSUE_NUM=$(echo "$extract-issue-number.output" | tr -d ...)

.archon/workflows/{test-workflows,experimental,maintainer}/*.yaml
  └─ bash smoke/release/review nodes: "$node.output" patterns

packages/workflows/src/defaults/bundled-defaults.generated.ts
  └─ embedded copy of default workflows

After

.archon/workflows/defaults/archon-fix-github-issue.yaml [~]
  └─ fetch-issue bash node [~]: RAW_ISSUE_NUM=$extract-issue-number.output

.archon/workflows/{test-workflows,experimental,maintainer}/*.yaml [~]
  └─ bash nodes [~]: assign substitutions unquoted, quote normal shell variables

packages/workflows/src/defaults/bundled-defaults.generated.ts [~]
  └─ regenerated embedded default workflow

Connection inventory (list every module-to-module edge, mark changes):

From To Status Notes
Default workflow YAML workflow validator modified archon-fix-github-issue no longer trips the bash double-quote warning.
Test/experimental/maintainer workflow YAML workflow validator modified Reported bash bodies use the safe unquoted-assignment idiom.
Default workflow YAML bundled defaults generated file modified bun run generate:bundled refreshed the embedded copy.

Label Snapshot

  • Risk: risk: low
  • Size: size: S
  • Scope: workflows|tests
  • Module: workflows:defaults, workflows:validator-fixtures

Change Metadata

  • Change type: bug
  • Primary scope: workflows

Linked Issue

Validation Evidence (required)

Commands and result summary:

bun run check:bundled
# Passed: bundled-defaults.generated.ts is up to date (36 commands, 20 workflows).

bun run cli validate workflows
# Exit 1 due existing unrelated validation issues:
# - archon-smart-pr-review missing .archon/mcp/ntfy.json
# - runtime availability warnings for bun/uv on this Windows shell
# Bash double-quote warnings are gone except the intentional e2e-structured-output-failfast negative smoke.

bun test packages/workflows/src/validator.test.ts
# Passed: 54 pass, 1 skip, 0 fail.

git diff --check HEAD^ HEAD
# Passed.

bun run validate
# Partial pass: check:bundled, check:bundled-skill, check:bundled-schema, type-check, lint, and format:check all passed.
# Failed during bun run test in an unrelated existing core test:
# packages/core/src/db/workflows.resume-cas.integration.test.ts throws ENOENT from SqliteAdapter(':memory:') mkdir on Windows.
# The same test fails when run alone.
  • Evidence provided (test/log/trace/screenshot): validator output confirms only the intentional negative smoke remains as a [bash] warning; bundled check and validator unit test output are included above.
  • If any command is intentionally skipped, explain why: None skipped.

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • New external network calls? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • File system access scope changed? (Yes/No) No
  • If any Yes, describe risk and mitigation: N/A

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Database migration needed? (Yes/No) No
  • If yes, exact upgrade steps: N/A

Human Verification (required)

What was personally validated beyond CI:

  • Verified scenarios: Ran workflow validation before and after the change; confirmed the shipped archon-fix-github-issue warning disappears and generated defaults are current.
  • Edge cases checked: Preserved the intentional e2e-structured-output-failfast negative smoke warning; did not edit non-bash when:, prompt:, or command: references.
  • What was not verified: Live execution of provider smoke workflows; those require provider credentials and are outside this YAML lint cleanup.

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows: bundled archon-fix-github-issue, tracked smoke workflows, experimental release/fix workflows, maintainer review workflow, generated bundled defaults.
  • Potential unintended effects: Minimal; the change keeps the same data flow but moves $node.output substitutions into ordinary shell variables before checks/greps/printfs.
  • Guardrails/monitoring for early detection: Existing workflow validator warns on any reintroduced "$node.output" bash body; check:bundled catches default bundle drift.

Rollback Plan (required)

  • Fast rollback command/path: git revert 7ae10e84e80a462972b2f5bd902189f974185085
  • Feature flags or config toggles (if any): None.
  • Observable failure symptoms: Workflow validation would again report [bash] double-quote warnings in the cleaned workflows, or affected smoke workflows would fail their non-empty output assertions.

Risks and Mitigations

  • Risk: Some smoke workflow output may contain whitespace or multiple lines.
    • Mitigation: Values are first captured via the unquoted Archon substitution, then passed to shell checks using quoted normal variables.

Summary by CodeRabbit

  • Bug Fixes

    • Improved issue and PR number extraction with more robust error handling and validation
  • Tests

    • Enhanced end-to-end test workflows with strengthened assertions and better variable handling for increased reliability

Avoid double-quoting pre-quoted node output substitutions in shipped, test, experimental, and maintainer workflow bash bodies. Refresh bundled default workflow output.

Closes coleam00#1895
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b2c78804-a0ec-4a6a-bdb6-87d8fba763ea

📥 Commits

Reviewing files that changed from the base of the PR and between ad0884b and 7ae10e8.

📒 Files selected for processing (16)
  • .archon/workflows/defaults/archon-fix-github-issue.yaml
  • .archon/workflows/e2e-opencode-all-nodes-smoke.yaml
  • .archon/workflows/e2e-opencode-inline-multi-agents.yaml
  • .archon/workflows/e2e-opencode-smoke.yaml
  • .archon/workflows/experimental/archon-fix-github-issue-experimental.yaml
  • .archon/workflows/experimental/archon-release.yaml
  • .archon/workflows/maintainer/maintainer-review-pr.yaml
  • .archon/workflows/test-workflows/e2e-claude-smoke.yaml
  • .archon/workflows/test-workflows/e2e-codex-smoke.yaml
  • .archon/workflows/test-workflows/e2e-copilot-all-nodes-smoke.yaml
  • .archon/workflows/test-workflows/e2e-deterministic.yaml
  • .archon/workflows/test-workflows/e2e-minimax-smoke.yaml
  • .archon/workflows/test-workflows/e2e-mixed-providers.yaml
  • .archon/workflows/test-workflows/e2e-pi-all-nodes-smoke.yaml
  • .archon/workflows/test-workflows/e2e-pi-smoke.yaml
  • packages/workflows/src/defaults/bundled-defaults.generated.ts

📝 Walkthrough

Walkthrough

This PR removes the bash $node.output double-quote footgun across 13 workflow files and regenerates the bundled defaults. Archon pre-quotes node outputs, so wrapping them in double quotes corrupts whitespace and special characters. Changes consolidate scattered variable-assignment anti-patterns into consistent explicit-variable capture patterns throughout e2e test and production workflows.

Changes

Workflow bash output quotation cleanup

Layer / File(s) Summary
Issue/PR number extraction refactoring
.archon/workflows/defaults/archon-fix-github-issue.yaml, .archon/workflows/experimental/archon-fix-github-issue-experimental.yaml, .archon/workflows/maintainer/maintainer-review-pr.yaml
fetch-issue and extract-issue-number steps refactored to store raw model output in intermediate variables (RAW_ISSUE_NUM, RAW_PR_NUM), then extract numeric sequences via grep instead of using tr character stripping. Error messages report the raw output for debugging.
Simple output quote removal (smoke tests)
.archon/workflows/test-workflows/e2e-claude-smoke.yaml, e2e-codex-smoke.yaml, e2e-mixed-providers.yaml, e2e-pi-smoke.yaml
Double quotes removed from $node.output variable assignments in assert steps, converting output="$simple.output" to output=$simple.output across four e2e smoke test workflows.
E2E assertion multiline refactoring
.archon/workflows/e2e-opencode-smoke.yaml, e2e-opencode-inline-multi-agents.yaml, e2e-opencode-all-nodes-smoke.yaml
Assert steps converted from single-line quoted bash to multiline blocks that capture upstream outputs into explicit variables (simple_output, multi_output, inline_output, prompt_output) before performing grep validations.
Comprehensive E2E assertion variable capture
.archon/workflows/test-workflows/e2e-copilot-all-nodes-smoke.yaml, e2e-pi-all-nodes-smoke.yaml
Substantial refactoring of assertion scripts to introduce intermediate variables for each upstream node's output (including structured nested fields like status and value), then pass those variables to validation functions instead of inline $node.output references.
Variable assignment pattern updates
.archon/workflows/test-workflows/e2e-deterministic.yaml, e2e-minimax-smoke.yaml, .archon/workflows/experimental/archon-release.yaml
Variable assignments updated to use intermediate shell variables (e.g., bash_echo, script_bun, version, bump, dry_run) either for template consistency or explicit variable management in validation/logging steps.
Bundled workflow regeneration
packages/workflows/src/defaults/bundled-defaults.generated.ts
Regenerated bundled TypeScript file after issue-number extraction refactoring, reflecting stricter output-format assumptions in archon-fix-github-issue workflow definition and corrected conditional expressions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • coleam00/Archon#1894: Introduced the validator warning and documentation for the bash $node.output double-quote footgun that this PR fixes across shipped and test workflows.
  • coleam00/Archon#1431: Introduced the e2e-minimax-smoke.yaml workflow that is updated in this PR to remove double-quote assignments.

Poem

🐰 The quotes were pre-wrapped, a footgun so sly,
With bash interpolation dreams that'd gone awry,
We stripped them clean, let variables breathe true,
Now workflows run steady—the test suite shines through! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix workflow bash output quoting' accurately describes the main change: removing double-quote anti-patterns from bash node substitutions in workflow YAML files.
Description check ✅ Passed The PR description follows the template thoroughly, covering all major sections including Summary, UX Journey, Architecture Diagram, validation evidence, security impact, compatibility, and rollback plan with detailed explanations.
Linked Issues check ✅ Passed The PR fully addresses issue #1895 by fixing bash double-quote patterns in default and test workflows, regenerating bundled defaults, preserving the intentional negative smoke test, and meeting all stated acceptance criteria.
Out of Scope Changes check ✅ Passed All changes are in-scope: workflow YAML files and their generated bundle. No unrelated validator/runtime logic, prompt/command text, or unrelated files were modified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up the bash $node.output double-quote footgun in shipped/checked-in workflows (follow-up to #1894)

1 participant