docs(#5789): add stdout contamination pattern to shell-scripting guide

[fullsend-ai-coder]

Summary

Add the stdout contamination in command substitution pattern to docs/contributing/shell-scripting.md, giving the review agent explicit guidance to catch functions that write diagnostic output to stdout when called inside $(...).

Related Issue

Closes #5789

Changes

• Add new "Stdout contamination in command substitution" section to docs/contributing/shell-scripting.md with problem description, anti-pattern/fix code examples, and review guidance paragraph following the existing format
• Update AGENTS.md table entry for Shell Scripting to reference the new pattern

Testing

• lychee markdown link check passes on both changed files
• gitlint passes on commit message
• scan-secrets passes on all changed files
• No tests to add — documentation-only change

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Commits are signed off (DCO) — human and human-directed agent sessions only
• I wrote this contribution myself and can explain all changes in it

---

Closes #5789

Post-script verification

• Branch is not main/master (agent/5789-stdout-contamination-pattern)
• Secret scan passed (gitleaks — 3f36f021def4d5a92e26200d3313dd26dba22cd0..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:11 AM UTC · Completed 7:25 AM UTC
Commit: 6d888b3 · View workflow run →

[github-actions]

Site preview

Preview: https://28da04f0-site.fullsend-ai.workers.dev

Commit: 982bc59ad148a7b0446e21df2d8fab4073010afe

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, which is a governance file requiring human approval. The change updates the Shell Scripting table entry to reference the new stdout contamination pattern. The PR is authorized by issue Add stdout-contamination-in-command-substitution pattern to shell-scripting.md #5789, and the AGENTS.md update is a necessary coherence fix to keep the index synchronized with guide content. Human approval is still required for protected-path changes.

Low

• [Technical documentation accuracy] docs/contributing/shell-scripting.md:82 — Both the WRONG and CORRECT example retry_curl() functions hardcode curl -fsSL "$@" in the function body, but the call sites also pass -fsSL again (resp=$(retry_curl -fsSL ...)), duplicating the flags. While harmless (curl ignores redundant short flags), this diverges from the actual action.yml pattern where retry_curl uses curl "$@" without hardcoded flags and callers pass flags explicitly. Either remove -fsSL from the function body to match the codebase convention, or remove it from the call sites.

---

Labels: PR adds new section to docs/contributing/shell-scripting.md and updates AGENTS.md table entry

[fullsend-ai-review]

See the review comment for full details.

[rh-hemartin]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 7:31 AM UTC · Completed 7:36 AM UTC
Commit: 6d888b3 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 1 of 2 review findings. Fixed the duplicated -fsSL flags in retry_curl documentation examples to match the codebase convention. The protected-path finding is informational and requires human approval, not a code fix.

Fixed (1):

1. Duplicated -fsSL flags in retry_curl examples: Removed hardcoded -fsSL from the curl command inside the retry_curl() function body in both the WRONG and CORRECT examples. The actual codebase convention in action.yml uses curl "$@" without hardcoded flags, with callers passing -fsSL explicitly. The examples now match this convention.

Disagreed (1):

1. protected-path: AGENTS.md requires human approval: This is informational — the review agent correctly notes AGENTS.md is a governance file requiring human approval. No code fix is needed; the PR already has the requires-manual-review label. Human approval is the appropriate resolution.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 9:04 AM UTC · Completed 9:18 AM UTC
Commit: 982bc59 · View workflow run →

[fullsend-ai-retro]

Retro: PR #5790 — docs(#5789): add stdout contamination pattern to shell-scripting guide

Workflow overview

This PR demonstrates the retro-to-merge feedback loop working as designed. The retro agent analyzed PR #5357 (where a human reviewer caught stdout contamination bugs that the review agent missed across 8 review runs), filed issue #5789, and the full pipeline executed cleanly: triage (4.5 min) → code (7 min) → review (15 min) → human-triggered fix (7 min) → human approval → merge. Total wall-clock ~2 hours, with ~82 minutes of human latency between fix completion and approval.

What went well

• Review quality was good. The review agent correctly identified a legitimate documentation accuracy issue: the code examples hardcoded -fsSL flags inside retry_curl() while call sites also passed -fsSL, diverging from the actual action.yml pattern. This was a useful, actionable finding.
• Fix agent handled the finding well. It fixed the code issue and correctly classified the protected-path AGENTS.md finding as informational (requiring human approval, not a code fix).
• The meta-loop closed. A gap identified by a prior retro (review agent missing stdout contamination in PR fix(version)!: resolve CLI version from reusable workflow ref #5357) was addressed by adding the pattern to the shell-scripting guide, strengthening future reviews.

Known issues observed (no new proposals needed)

Missing re-review after fix push. After the fix agent pushed commit 982bc59, no automated re-review was triggered. The synchronize event path in reusable-dispatch.yml requires is_event_actor_authorized(), which fails for bot accounts since GitHub App bots have no collaborator permissions. The initial review only worked because the ready-for-review label path has no auth gate. This is well-covered by existing issues: #5188 (root cause — permission check failing for bots), #5666 (route fix-agent synchronize events to review), #5706 (add bot carve-out to dispatch path), and #897 (fix agent should re-request review). This retro provides additional evidence: run 30613413666 hit the "No stage matched" path after the fix push.

Code agent did not cross-reference actual source code when writing examples. The -fsSL duplication arose because the code agent wrote examples without checking the actual retry_curl() implementation in action.yml. This is covered by fullsend-ai/agents#387 (verify documentation claims against source files) and fullsend-ai/agents#436 (cross-reference existing content when adding documentation sections).

Conclusion

Clean workflow with no new proposals. All identified improvement opportunities are already tracked in existing issues.