Skip to content

[code-simplifier] refactor: simplify guard policy setOutput, footer logic, and comment clarity#22016

Merged
pelikhan merged 1 commit intomainfrom
code-simplifier/2026-03-20-38fa8ede606fa8ea
Mar 20, 2026
Merged

[code-simplifier] refactor: simplify guard policy setOutput, footer logic, and comment clarity#22016
pelikhan merged 1 commit intomainfrom
code-simplifier/2026-03-20-38fa8ede606fa8ea

Conversation

@github-actions
Copy link
Contributor

This PR simplifies recently modified code (2026-03-20) to improve clarity and reduce duplication while preserving all functionality.

Files Simplified

  • actions/setup/js/determine_automatic_lockdown.cjs — eliminate duplicate core.setOutput() calls
  • actions/setup/js/pr_review_buffer.cjs — simplify shouldAddFooter initialization logic
  • pkg/workflow/mcp_environment.go — clarify misleading duplicate comment

Improvements Made

1. Reduced Code Duplication (determine_automatic_lockdown.cjs)

The setOutput() call was duplicated in both branches of an if/else (once for the "not configured" case, once for the "already configured" case). Since both branches set the exact same computed value (resolvedMinIntegrity / resolvedRepos), the call is now moved outside the conditional — keeping only the distinct core.info() messages inside.

Before:

if (!configuredMinIntegrity) {
  core.info(`...`);
  core.setOutput("min_integrity", defaultMinIntegrity);
} else {
  core.info(`...`);
  core.setOutput("min_integrity", configuredMinIntegrity);
}

After:

if (!configuredMinIntegrity) {
  core.info(`...`);
} else {
  core.info(`...`);
}
core.setOutput("min_integrity", resolvedMinIntegrity);

2. Simplified Boolean Logic (pr_review_buffer.cjs)

shouldAddFooter was initialized to false with a redundant else if (footerMode === "none") { shouldAddFooter = false; } branch. Now initialized directly from the "always" check:

Before:

let shouldAddFooter = false;
if (footerMode === "always") { shouldAddFooter = true; }
else if (footerMode === "none") { shouldAddFooter = false; }
else if (footerMode === "if-body") { ... }

After:

let shouldAddFooter = footerMode === "always";
if (footerMode === "if-body") { ... }

3. Clarified Comment (mcp_environment.go)

The comment // Check for safe-outputs env vars appeared twice in collectMCPEnvironmentVariables. The second occurrence was misleading — that block adds the safe-outputs server connection details (port and API key), not the GH_AW_SAFE_OUTPUTS passthrough above it. Clarified to // Add safe-outputs server connection env vars.

Changes Based On

Recent merged PRs:

Testing

  • ✅ JS tests pass — 53/53 (determine_automatic_lockdown.test.cjs, pr_review_buffer.test.cjs)
  • ✅ Go build succeeds (make build)
  • ✅ Go tests pass (TestCollectMCPEnvironmentVariables_*, TestBuildSafeOutputsSection, TestCreateWorkflowTemplateContainsOnlyValidSafeOutputs)
  • make fmt — no formatting changes needed
  • ✅ No functional changes — behavior is identical

References: §23357917108

Note

🔒 Integrity filtering filtered 4 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

Generated by Code Simplifier ·

  • expires on Mar 21, 2026, 7:02 PM UTC

…clarity

- determine_automatic_lockdown.cjs: eliminate duplicate core.setOutput()
  calls in if/else branches by moving them after the conditional, using
  the already-computed resolvedMinIntegrity/resolvedRepos variables
- pr_review_buffer.cjs: simplify shouldAddFooter initialization — derive
  directly from footerMode === 'always' and remove the redundant
  shouldAddFooter = false branch for 'none'
- mcp_environment.go: clarify the second 'Check for safe-outputs env vars'
  comment that was identical to the one above; this block adds server
  connection details (port/API key), not the GH_AW_SAFE_OUTPUTS passthrough

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review March 20, 2026 19:05
Copilot AI review requested due to automatic review settings March 20, 2026 19:05
@pelikhan pelikhan merged commit 24bc122 into main Mar 20, 2026
@pelikhan pelikhan deleted the code-simplifier/2026-03-20-38fa8ede606fa8ea branch March 20, 2026 19:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs small refactors to simplify control flow and reduce duplication while keeping behavior unchanged across guard-policy output computation, PR review footer logic, and safe-outputs environment variable comments.

Changes:

  • Deduplicates core.setOutput() calls in automatic guard policy resolution by computing resolved values once and emitting outputs after branch-specific logging.
  • Simplifies shouldAddFooter initialization in PR review submission logic by deriving the default from footerMode === "always".
  • Clarifies a misleading duplicated comment in MCP environment variable collection related to safe-outputs server connection variables.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
actions/setup/js/determine_automatic_lockdown.cjs Moves repeated setOutput calls outside conditionals while preserving resolved values and logging.
actions/setup/js/pr_review_buffer.cjs Simplifies footer inclusion boolean initialization without changing supported modes.
pkg/workflow/mcp_environment.go Updates comment text to accurately describe the safe-outputs server env vars being added.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants