Address pre-existing Copilot review comments surfaced by PR #1212 (workflow permissions, logic bugs, YAML escaping) #1213

Background

PR #1212 (Phase 0 sync-back of main → dev for v0.10.0 release) received 12 inline review comments from the Copilot reviewer bot. Investigation showed all 12 are on pre-existing code on main (first added between 2026-04-03 and 2026-04-21), and PR #1212 merely surfaces them to dev for the first time. They are out of scope for Phase 0 (which is a mechanical merge with zero source edits beyond conflict resolution).

This issue tracks them for a follow-up PR.

Comments to address

CI workflow permissions (likely cause 403s at runtime)

1. .github/workflows/squad-pr-nudge.yml:11 — permissions block has issues: read but the workflow calls github.rest.issues.createComment(...) which needs issues: write. Will 403 when posting nudge.
2. .github/workflows/squad-impact.yml:14 — calls github.rest.issues.updateComment/createComment but lacks issues: write. Upserting the impact report will fail.
3. .github/workflows/squad-repo-health.yml:14 — calls github.rest.issues.*Comment via scripts/repo-health-comment.mjs but grants only pull-requests: write, not issues: write.

CI workflow logic bugs

4. .github/workflows/squad-pr-readiness.yml:49 — PR_LABELS built with toJson(github.event.pull_request.labels || '[]'). On workflow_run events github.event.pull_request.labels is null, so the fallback is the string '[]' and toJson('[]') produces the literal string "[]", not a parseable JSON array.
5. .github/workflows/squad-pr-nudge.yml:151 — compareCommits() check uses ahead_by but the messaging says "commits behind". Should be behind_by. Causes incorrect nudges for branches that are ahead of base.

scripts/pr-readiness.mjs

6. L247 — checkCIStatus() treats legacy combined statuses as "all passing" when no check-runs exist, ignoring failure/error states in the combined status. False positives possible.
7. L316 — truncation row for files.length > MAX_FILE_LIST adds an extra column (| ... | **+N more files** | |), producing a malformed 3-column row in a 2-column table.
8. L448 — PR_LABELS parsed via JSON.parse(...) without array validation. Non-array values will throw on later .map()/.filter() calls.

scripts/check-bootstrap-deps.mjs

9. L50 — isNodeBuiltin() returns true for any specifier starting with node: including non-existent ones like node:does-not-exist, defeating the verification gate.

packages/squad-cli/src/cli/commands/skill.ts

10. L152, L156, L201 — Generated YAML uses description: "" without escaping. If a skill description contains " or \n, the emitted apm.yml becomes invalid YAML. Recommendation: use JSON.stringify(desc) (JSON strings are YAML-superset-compatible) or a YAML library's quoting helper.

Severity & priority

• Workflow permission bugs (feat: worktree awareness, Scribe auto-commit, and decision consolidation #1, feat: GitHub Issues intake, PRD mode, and human team members #2, [TEST] API capability test — will be deleted #3): high impact — these workflows will silently fail to post comments. Fix soon.
• Logic bug ahead_by vs behind_by (feat: Issue Assignment & Auto-Triage for Squad Members #5): high — will incorrectly nudge ahead-of-base PRs.
• YAML escaping (Copilot client parity: Squad works fully only on GitHub Copilot CLI #10): medium — affects any skill with quotes/newlines in description. Edge case but real.
• Others: medium to low — defensive coding, false-positive paths.

Recommended follow-up

Open a single PR after v0.10.0 ships that addresses all 12. They're small, surgical fixes; suggest grouping into one PR to limit review surface. The CI workflow permission fixes (#1-#3) are 3-line YAML additions, lowest risk.

Why not fix in PR #1212?

PR #1212 is a Phase 0 sync-back for v0.10.0 release. Its scope is purely mechanical: bring main into dev so dev → preview → main will see a fast-forward at the end. Adding unrelated code fixes would:

1. Violate the scope-drift guideline in .github/copilot-instructions.md
2. Expand the diff surface and risk introducing new bugs during a release window
3. Break the clean audit trail (Phase 0 = sync only)

These bugs have lived on main for 6–8 weeks; another few days while v0.10.0 ships is the right tradeoff.

Filed during v0.10.0 release execution. Refs PR #1212.