Skip to content

refactor: narrow convertToGitHubActionsEnv env parameter from any to map[string]any - #48992

Merged
pelikhan merged 3 commits into
mainfrom
copilot/refactor-narrow-any-fields
Jul 30, 2026
Merged

refactor: narrow convertToGitHubActionsEnv env parameter from any to map[string]any#48992
pelikhan merged 3 commits into
mainfrom
copilot/refactor-narrow-any-fields

Conversation

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

convertToGitHubActionsEnv accepted env any and immediately performed a runtime type assertion to map[string]any, with a silent no-op fallback for any other type. All three call sites always pass server.Config["env"] which is typed any but is always a map[string]any in practice.

Changes

  • pkg/cli/actions.go — narrows parameter from env any to env map[string]any; removes the env.(map[string]any) assertion wrapper (iterating a nil map is a no-op in Go, so nil safety is preserved).
  • pkg/cli/mcp_add.go — the three call sites now assert server.Config["env"] to map[string]any at the boundary before passing it in, making the type contract explicit where the any originates.
  • pkg/cli/actions_test.go — narrows input field from any to map[string]any; removes the non-map input returns empty map case, which is now statically impossible.
// Before
func convertToGitHubActionsEnv(env any, ...) map[string]string {
    if envMap, ok := env.(map[string]any); ok {  // silent no-op if wrong type
        for key, value := range envMap { ... }
    }
    return result
}

// After
func convertToGitHubActionsEnv(env map[string]any, ...) map[string]string {
    for key, value := range env { ... }  // nil-safe; type guaranteed at call site
    return result
}

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.8 AIC · ⌖ 5.8 AIC · ⊞ 7.8K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.3 AIC · ⌖ 8.85 AIC · ⊞ 7.8K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30503727864

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 5.5 AIC · ⌖ 8.58 AIC · ⊞ 7.8K ·
Comment /souschef to run again

…map[string]any

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor safe_output_handlers.go and actions.go 'any' fields to concrete types refactor: narrow convertToGitHubActionsEnv env parameter from any to map[string]any Jul 29, 2026
Copilot AI requested a review from pelikhan July 29, 2026 22:18
@pelikhan
pelikhan marked this pull request as ready for review July 29, 2026 22:28
Copilot AI review requested due to automatic review settings July 29, 2026 22:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Narrows environment conversion to a concrete Go map type, improving compile-time type safety.

Changes:

  • Updates the converter signature and removes its runtime assertion.
  • Moves type assertions to configuration boundaries.
  • Aligns unit tests with the narrowed contract.
Show a summary per file
File Description
pkg/cli/actions.go Narrows the converter input type.
pkg/cli/mcp_add.go Validates environment maps at call sites.
pkg/cli/actions_test.go Updates tests for the concrete input type.

Review details

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Medium

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

Run: https://github.com/github/gh-aw/actions/runs/30497974805

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.8 AIC · ⌖ 5.8 AIC · ⊞ 7.8K ·
Comment /souschef to run again

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

Run: https://github.com/github/gh-aw/actions/runs/30497974805...

PR finisher complete. Here's the status:

  • ✅ Reviews — copilot-pull-request-reviewer found 0 issues. No unresolved review threads.
  • ✅ Checks (local) — make fmt, make lint, and make test-unit-all all pass with no failures.
  • ❓ Checks (CI) — Stale after agent push; needs human re-trigger. The action_required conclusions on CGO, CWI, Doc Build, and similar workflows are environment-protection gates waiting for maintainer approval, not code failures. The copilot check is the current agent run.
  • ✅ Mergeable — PR is open, not draft, and branch is up to date with main (merge commit already applied).

Actions taken: Local validation confirmed clean; no code changes were needed.
Hand-off: A maintainer needs to approve the environment-gated checks and re-trigger CI before merge.

Copilot AI requested a review from gh-aw-bot July 29, 2026 23:21
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, review the latest PR status, and rerun any needed checks once the branch is current. If no code changes are needed, summarize why this PR is ready for maintainer review and call out any remaining human-only actions.

Run: https://github.com/github/gh-aw/actions/runs/30499690701

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.3 AIC · ⌖ 8.85 AIC · ⊞ 7.8K ·
Comment /souschef to run again

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, review the latest PR status, and rerun any needed checks once the branch is current. If no code changes are needed, summarize why this PR is ready for main...

This PR is ready for maintainer review. Here's the current status:

  • Reviews — Zero review threads open. copilot-pull-request-reviewer raised no issues.
  • Checks (local)make fmt, make lint, and make test-unit-all all pass (confirmed in the previous pass; no code has changed since).
  • Checks (CI) — The previous copilot check run (run 30498452780) completed with success. The action_required conclusions on CGO, CWI, and Doc Build are environment-protection gates that require explicit maintainer approval — they are not code failures. CI is stale on the agent's HEAD and needs a human re-trigger before merge.
  • Mergeable — PR is open, not draft, and branch is current with main (merge commit ac896a3 already applied).

Why this PR is ready: The refactor is a pure type-safety improvement with no behavioral change. It narrows env anyenv map[string]any in one internal function, moves the type assertion to the three call sites where the any originates, and removes a now-statically-impossible test case. All local checks pass.

Remaining human-only actions:

  1. Approve the environment-gated required checks (CGO, CWI, Doc Build).
  2. Re-trigger CI (close/reopen, workflow_dispatch, or push) to get a fresh green run.
  3. Merge once CI is green.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, review the latest feedback on this PR, address any unresolved reviewer requests, and rerun checks once the branch is up to date. Run context: https://github.com/github/gh-aw/actions/runs/30503727864

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 5.5 AIC · ⌖ 8.58 AIC · ⊞ 7.8K ·
Comment /souschef to run again

@github-actions

Copy link
Copy Markdown
Contributor

Triage: refactor: narrow convertToGitHubActionsEnv env param from any to map

  • Category: refactor | Risk: low | Score: 40/100 (Impact 15, Urgency 10, Quality 15)
  • Recommendation: batch_review (batch: low-risk-cleanup) — small type-safety refactor, CI green, 3 files. Group with similar low-risk cleanup PRs for a single review pass.

Generated by 🔧 PR Triage Agent · aut00 · 52.7 AIC · ⌖ 9.26 AIC · ⊞ 7.9K ·

Copilot AI commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, review the latest feedback on this PR, address any unresolved reviewer requests, and rerun checks once the branch is up to date. Run context: https://githu...

No changes needed — PR state is unchanged from the previous two passes:

  • Reviews — Zero open review threads; no unresolved feedback.
  • Checks (local)make fmt, make lint, and make test-unit-all pass (verified in prior passes; no code has changed since).
  • Checks (CI) — All three prior copilot check runs completed success. The unstable merge state is from environment-protection gates (CGO, CWI, Doc Build) that require explicit maintainer approval — not code failures. CI on the current HEAD needs a human re-trigger.
  • Mergeable — Open, not draft, branch is current with main.

Still needed (human-only):

  1. Maintainer approval of the environment-gated required checks.
  2. CI re-trigger (close/reopen or push from a maintainer) to get a fresh green run.
  3. Merge.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, review the latest PR status, and if no code changes are needed, summarize why this PR is ready for maintainer review and call out any remaining human-only actions.

Run: https://github.com/github/gh-aw/actions/runs/30507870059

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.44 AIC · ⌖ 7.77 AIC · ⊞ 7.8K ·
Comment /souschef to run again

Copilot AI commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, review the latest PR status, and if no code changes are needed, summarize why this PR is ready for maintainer review and call out any remaining human-only ...

Still no code changes needed. Current status:

  • Reviews — Zero open review threads; copilot-pull-request-reviewer raised no issues.
  • Checks (local)make fmt, make lint, make test-unit-all all pass (verified on earlier passes; no code has changed since).
  • Checks (CI) — All four prior copilot check runs concluded success. The unstable merge state is caused solely by environment-protection gates (CGO, CWI, Doc Build) requiring explicit maintainer approval — not code failures. CI on the current HEAD is unverified until a human re-triggers it.
  • Mergeable — Open, not draft, branch is current with main.

Remaining human-only actions:

  1. Approve the environment-gated required checks (CGO, CWI, Doc Build).
  2. Re-trigger CI (close/reopen or maintainer push) for a fresh run.
  3. Merge once CI is green.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, review the latest PR status, and if no code changes are needed, summarize why this PR is ready for maintainer review and call out any remaining human-only actions.

Run: https://github.com/github/gh-aw/actions/runs/30514455658

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.4 AIC · ⌖ 5.75 AIC · ⊞ 7.8K ·
Comment /souschef to run again

Copilot AI commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, review the latest PR status, and if no code changes are needed, summarize why this PR is ready for maintainer review and call out any remaining human-only ...

No code changes needed. This PR is ready for maintainer review:

  • Reviews — Zero open review threads; copilot-pull-request-reviewer raised no issues.
  • Checks (local)make fmt, make lint, make test-unit-all all pass (verified across multiple prior passes; no code has changed).
  • Checks (CI) — All five prior copilot check runs concluded success. The unstable merge state is caused solely by environment-protection gates (CGO, CWI, Doc Build) requiring explicit maintainer approval — not code failures. CI on the current HEAD is unverified until a human re-triggers it.
  • Mergeable — Open, not draft, branch is current with main.

Remaining human-only actions:

  1. Approve the environment-gated required checks (CGO, CWI, Doc Build).
  2. Re-trigger CI (close/reopen or maintainer push) for a fresh green run.
  3. Merge once CI is green.

@pelikhan
pelikhan merged commit 625f00a into main Jul 30, 2026
34 checks passed
@pelikhan
pelikhan deleted the copilot/refactor-narrow-any-fields branch July 30, 2026 06:30
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.

[Code Quality] refactor: narrow safe_output_handlers.go and actions.go 'any' fields to concrete types

4 participants