docs(#5422): add yq/jq pitfalls and fail-open gate severity guidance

[fullsend-ai-coder]

Summary

Adds two new subsections to the AGENTS.md shell scripting section to address guidance gaps identified in PR #4049, where a jq-only function (ascii_downcase) was used in a yq expression and the surrounding 2>/dev/null || echo "" pattern silently made the agent-check gate non-functional.

Changes

• yq/jq pitfalls — documents common function name mismatches between yq and jq (downcase vs ascii_downcase, upcase vs ascii_upcase), instructs agents to verify functions against yq's built-in set, and sets reviewer severity at medium
• Fail-open error suppression — distinguishes gate/guard steps (where fail-open is a correctness bug) from non-critical steps (where it's acceptable), includes code examples of the dangerous pattern and a safer fail-closed alternative, and instructs reviewers to flag fail-open in gate steps at medium severity or higher

Testing

• make lint-md-links passes — no broken markdown links introduced
• Markdown renders correctly with proper heading hierarchy (H3 subsections under H2 Shell scripting)
• Monitor next 3 code agent PRs with yq expressions for correct function names (per issue validation criteria)
• Monitor next 3 review agent runs on fail-open patterns in gate steps for correct severity (per issue validation criteria)

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Documentation-only change — no code or test changes needed

---

Closes #5422

Post-script verification

• Branch is not main/master (agent/5422-yq-failopen-guidance)
• Secret scan passed (gitleaks — 129d28b4cf53342aaf871e45cc45845f6a1ab61d..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 8:18 PM UTC · Completed 8:33 PM UTC
Commit: 29e2b03 · View workflow run →

[github-actions]

Site preview

Preview: https://8b248d2a-site.fullsend-ai.workers.dev

Commit: 0a74f6f947959e415ae10ee91288f0d8a4d8d354

[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 — AGENTS.md is a protected path. This PR modifies it with justification (linked issue AGENTS.md: add yq function validation and fail-open gate severity guidance to shell scripting section #5422, which explains the guidance gaps identified in PR feat(#4026): support disabling agents via config.yaml enabled field #4049). Human approval is always required for protected-path changes, regardless of context.

Previous run

Review

Findings

Medium

• [protected-path] AGENTS.md — AGENTS.md is a protected path. This PR modifies it with justification (linked issue AGENTS.md: add yq function validation and fail-open gate severity guidance to shell scripting section #5422, which explains the guidance gaps identified in PR feat(#4026): support disabling agents via config.yaml enabled field #4049). Human approval is always required for protected-path changes, regardless of context.

Low

• [technical accuracy] AGENTS.md:85 — The claim "split in yq requires different syntax than jq's split for some input types" is imprecise. The basic split("delimiter") syntax is identical in both tools; the actual difference is that jq supports a 2-arity split(regex; flags) form that yq does not. The phrase "for some input types" suggests the difference is about the data being split rather than function arity.
  Remediation: Replace with a more precise description, e.g.: "ltrimstr/rtrimstr exist in both. split(\"str\") works identically in both, but jq's 2-arity split(regex; flags) form is not available in yq."

---

Labels: PR modifies AGENTS.md documentation with shell scripting guidance

[waynesun09]

Automated multi-agent review (3 independent passes, cross-checked against actual yq/jq behavior rather than by inspection alone). One new medium-severity finding on the "yq/jq pitfalls" section, plus an update to an existing low-severity comment:

• The "silently fails / produces no output" framing for unrecognized yq functions doesn't match actual behavior (loud parse error, non-zero exit) - see inline comment.
• Updated the earlier low-severity comment on the ltrimstr/rtrimstr/split bullet to medium: ltrimstr/rtrimstr don't exist in yq at all (the original claim has it backwards), which is a more significant inaccuracy than the split note alone.

The fail-open guidance and its bash examples checked out against testing.

[rh-hemartin]

@waynesun09 how do we proceed here? I think it that for some reason is not happening. Also this has no assignee and the issue is a retro filed one with no assignee either.

[waynesun09]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 6:58 PM UTC · Completed 7:03 PM UTC
Commit: 7ada4e0 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 3 review findings (2 inline comments covering 3 inaccuracies). Corrected the 'silently fails' framing to accurately describe yq's loud parse-error behavior, fixed the ascii_downcase bullet, and replaced the backwards ltrimstr/rtrimstr claim with accurate information. All changes are documentation-only in AGENTS.md.

Fixed (3):

1. silently fails framing does not match actual yq behavior (AGENTS.md): Replaced 'silently fails, producing empty or incorrect output' with accurate description: yq causes a parse error (non-zero exit, stderr message) on unrecognized functions, and it becomes silent only when combined with error suppression. Added cross-reference to the Fail-open error suppression section.
2. ascii_downcase silently produces no output claim is wrong (AGENTS.md): Changed 'silently produces no output' to 'causes a parse error' for the ascii_downcase bullet, matching verified yq v4.47.2 behavior.
3. ltrimstr/rtrimstr exist in both is backwards and split divergence is imprecise (AGENTS.md): Replaced incorrect claim that ltrimstr/rtrimstr exist in both tools. They are jq-only; yq only has trim (whitespace-only). Clarified split divergence: split("str") works the same in both, but jq's 2-arg split(regex; flags) form has no yq equivalent.

Tests: passed

_{Updated by fullsend fix agent}

[waynesun09]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:45 AM UTC · Completed 11:57 AM UTC
Commit: 7ada4e0 · View workflow run →

[waynesun09]

Final confirmation squad pass (3 agents: Claude, Claude, Gemini) after rebasing onto main (content moved from AGENTS.md to docs/contributing/shell-scripting.md during main's doc restructure) and applying the in-scope fixes from the prior review round (split() semantics claim corrected, SAFER-example nuance added, severity-rule caveat added).

All technical claims independently re-verified against real yq v4.47.2 / jq 1.7.1 binaries - no remaining inaccuracies. One self-contradiction the fix introduced (bolded "fail-closed" callout directly contradicted by the very next paragraph) was caught and corrected in a follow-up commit. CI is fully green, no conflicts.

The two pre-existing workflow bugs found in .github/workflows/reusable-dispatch.yml (fail-open gate steps predating this PR) remain out of scope per explicit direction - not fixed here, no follow-up issue filed.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 4:25 PM UTC · Completed 4:35 PM UTC
Commit: 0a74f6f · View workflow run →

[fullsend-ai-retro]

Retro: PR #5423 — yq/jq pitfalls and fail-open gate severity guidance

Workflow: Issue #5422 (retro-filed from PR #4049) → triage (12 min) → code agent → PR #5423 → review → fix → re-review → human approval → merged. Total: ~44 hours, with the automated pipeline (issue → PR) completing in ~12 minutes and the remaining time spent awaiting human review.

What went well:

• The review agent caught a genuine factual error (the ltrimstr/rtrimstr reversal) through independent verification against yq v4.47.2, and self-corrected its initial LOW severity to MEDIUM after 3-agent cross-checking.
• The fix agent (run 29948918482) performed well — it corrected all 3 technical inaccuracies and produced more accurate text than the review agent's own suggested remediation.
• waynesun09's final approval used a 3-agent confirmation squad (Claude, Claude, Gemini) that independently verified all claims against real binaries, catching a self-contradiction the fix agent introduced.
• The re-review (run 30004227474) confirmed 0 new findings across all dimensions.

What required rework:

• The retro agent's issue AGENTS.md: add yq function validation and fail-open gate severity guidance to shell scripting section #5422 contained an inaccurate claim that yq "silently fails" on jq-only functions. In reality, yq produces a loud parse error with non-zero exit — the silence in PR feat(#4026): support disabling agents via config.yaml enabled field #4049 came from the 2>/dev/null || echo "" wrapper, not from yq itself. The code agent propagated this framing into the documentation.
• The review agent (run 29865101061) caught the peripheral ltrimstr/rtrimstr error but missed the central "silently fails" premise, which was the main technical claim of the new section. waynesun09 caught this with direct testing.

Evidence for existing issues (skipped as proposals):

• Code agent should verify documentation claims against source files during generation agents#387 (code agent should verify documentation claims against source files during generation): PR docs(#5422): add yq/jq pitfalls and fail-open gate severity guidance #5423 provides a second instance — the code agent propagated the retro agent's "silently fails" claim without verifying yq's actual behavior. This corroborates the pattern identified in fix: remove pull_request_review trigger from shim to stop review loop #387 from PR docs(#5458): document full mintcore file lifecycle in AGENTS.md #5459.
• Review agent should fact-check technical claims in documentation PRs #2199 (review agent should fact-check technical claims in documentation PRs): The review agent verified ltrimstr/rtrimstr behavior but did not verify the "silently fails" claim despite having the same verification capability. This is another data point for Review agent should fact-check technical claims in documentation PRs #2199.
• Review correctness sub-agent should cross-reference verified facts to detect misleading documentation narratives agents#265 (correctness sub-agent should detect misleading documentation narratives): The PR's own description attributed silence to the 2>/dev/null pattern, while the documentation text blamed yq itself — an internal contradiction the review agent did not synthesize. This is precisely the narrative-consistency gap SSRF: TOCTOU DNS rebinding race between hook validation and tool execution #265 proposes to close.
• Review agent should validate yq/jq expression correctness in workflow YAML files agents#344 (review agent should validate yq/jq expression correctness): This companion issue from the same PR feat(#4026): support disabling agents via config.yaml enabled field #4049 retro addresses the capability-level gap. PR docs(#5422): add yq/jq pitfalls and fail-open gate severity guidance #5423 is the documentation counterpart.

1 proposal filed targeting the retro agent's causal attribution accuracy when filing issues.

Proposals filed

• Retro agent should decompose multi-layer failure modes rather than attributing composite behavior to a single component (in fullsend-ai/agents)