[pelikhan]

Summary

Add an Allow intentional protected-file changes section to generated protected-file warnings in pull requests, reviews, fallback issues, and failure reports.

The examples show how to allow .changeset/* using safe-outputs.<operation>.protected-files.exclude: [".changeset/"], without disabling protection for other files. The guidance explains folder-prefix matching (including subfolders), preserving existing policies and exclusions, independent allowed-files restrictions, and recompiling the workflow.

Adds regression coverage for typed report blocks and rendered guidance across the warning templates and affected safe-output operations. Motivated by the protected-files notice in #72874; no protection behavior is changed.

Validation

• TMPDIR=/tmp make agent-report-progress — passed.
• TMPDIR=/tmp npm --prefix actions/setup/js run test:js -- --no-file-parallelism create_pull_request_helpers.test.cjs handle_agent_failure.test.cjs create_pull_request.test.cjs push_to_pull_request_branch.test.cjs — passed (1,212 tests).
• make fmt-cjs && make lint-cjs — passed.
• npm --prefix actions/setup/js run typecheck — passed.

TMPDIR=/tmp aligns the existing bundle recovery tests with their generated /tmp artifact paths on macOS. The final gate reported existing lint warnings but no errors.

[Copilot]

🟡 Changes recommended

The change still has an outdated snapshot that will fail tests and leaves the push-failure fallback template without the new scoped-exclusion guidance.

2 open issues

• Update messages preview snapshot for protected-file guidance · New
• Cover push-failure fallback template with exclusion guidance · New

What changed in this PR

This pull request adds guidance for intentionally excluding protected files from safe-output protection warnings without changing protection behavior.

Changes:

• Added protected-files.exclude examples and explanations to review notices and fallback issue templates.
• Added scoped-exclusion guidance to protected-file failure reports.
• Added regression coverage for rendered templates, typed report blocks, and operation-specific YAML snippets.

Review findings:

• Critical (3 votes): The message preview snapshot must be regenerated to reflect the updated protected-file templates.
• Nit (2 votes): The push-failed fallback template remains without the new scoped-exclusion guidance and should be added to the regression matrix.
• Nit (1 vote): Guidance currently implies generic folder-prefix matching, although the compiled configuration does not support arbitrary prefixes; the wording should be narrowed or matching implemented.
• Nit (1 vote): The same folder-prefix wording issue applies to the push-to-PR fallback template.
• Nit (1 vote): The same folder-prefix wording issue applies to the request-changes review template.
• Nit (1 vote): The same folder-prefix wording issue applies to the request-review template.
• Nit (1 vote): The rendered-template coverage matrix omits manifest_protection_push_failed_fallback.md.

File	Description
actions/​setup/​md/​manifest_protection_request_review.md	Adds exclusion guidance to request-review pull request notices.
actions/​setup/​md/​manifest_protection_request_changes_review.md	Adds exclusion guidance to request-changes review content.
actions/​setup/​md/​manifest_protection_push_to_pr_fallback.md	Adds exclusion guidance to push fallback issues.
actions/​setup/​md/​manifest_protection_create_pr_fallback.md	Adds exclusion guidance to create-PR fallback issues.
actions/​setup/​js/​handle_agent_failure.cjs	Builds typed exclusion guidance for protected-file failure reports.
actions/​setup/​js/​handle_agent_failure.test.cjs	Verifies rendered failure-report guidance and operation-specific snippets.
actions/​setup/​js/​create_pull_request_helpers.test.cjs	Verifies typed template blocks and rendered guidance across protected-file templates.

_{Review effort: Lite (auto)}

[github-actions]

Skills-Based Review 🧠

Applied /grill-with-docs and /codebase-design — approving with no blocking issues.

📋 Review notes

What I checked

• Traced the runtime matching logic end-to-end: manifest_file_helpers.cjs (checkForProtectedPaths, startsWith prefix match), splitProtectedFilesExcludes in pkg/workflow/safe_outputs_config_generation.go, and config_preprocessing.go's object-form protected-files: { policy, exclude } parsing. The new guidance text ("folder exclusions use path prefixes, not glob patterns, and allow all subfolders") is accurate — it matches startsWith(prefix) semantics, not globPatternToRegex glob matching used by allowed-files/excluded-files.
• Confirmed the allowed-files interaction claim is correct: checkFileProtection in manifest_file_helpers.cjs runs the allowlist check (step 1) independently of the protected-files check (step 2) — both must pass, so the guidance's "these paths must also match it" wording is right.
• Verified all four Markdown templates (manifest_protection_create_pr_fallback.md, manifest_protection_push_to_pr_fallback.md, manifest_protection_request_review.md, manifest_protection_request_changes_review.md) consistently substitute the correct safe-output key (create-pull-request vs push-to-pull-request-branch) and correct policy: value (fallback-to-issue / request-review) per template — no copy-paste drift found.
• New it.each test in create_pull_request_helpers.test.cjs parametrizes over all four templates and asserts the rendered body contains the key phrases (.changeset/*, "not glob patterns", allowed-files, gh aw compile) and does not regress to contain protected-files: allowed — good regression coverage per /tdd principles (specification-style assertions, each template gets its own case via it.each).
• handle_agent_failure.cjs's new <details> block follows the same pattern as the existing ⚙️ Configure protected-files: fallback-to-issue block, consistent with /codebase-design's guidance to reuse existing structural patterns rather than introduce a new one.

Minor observation (non-blocking)

• manifest_protection_request_changes_review.md is the only one of the four templates that does not wrap the new guidance in a <details> collapsible section (the other three do). This is a minor inconsistency in presentation, not correctness — the content itself is accurate and test-covered. Not requesting a change since it may be intentional given the review template's already-compact structure.

Positive highlights

• ✅ Dynamic YAML snippet in handle_agent_failure.cjs builds excludeSnippet per blocked safe-output type, keeping the example aligned with what actually triggered the failure, rather than a generic hardcoded example.
• ✅ Good terminology precision: explicitly distinguishing "path prefixes" from "glob patterns" prevents a likely user mistake (trying .changeset/* as the exclude value, which would not match via startsWith).
• ✅ No protection behavior changed — purely additive documentation in generated reports, confirmed by code symmetry (no new config fields introduced, only explanatory nodes.details(...) blocks).

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~141 AIC · ⌖ ~15 AIC · ⊞ 10.2K
_{Comment /matt to run again}