Skip to content

refactor: split safe_outputs_config.go into focused modules#20639

Merged
pelikhan merged 3 commits intomainfrom
copilot/refactor-safe-outputs-config
Mar 12, 2026
Merged

refactor: split safe_outputs_config.go into focused modules#20639
pelikhan merged 3 commits intomainfrom
copilot/refactor-safe-outputs-config

Conversation

Copy link
Contributor

Copilot AI commented Mar 12, 2026

pkg/workflow/safe_outputs_config.go had grown to 1077 lines spanning three unrelated domains: frontmatter extraction, GitHub App token lifecycle, and messages/mentions parsing.

Split

File Lines Responsibility
safe_outputs_config.go 605 Frontmatter extraction (extractSafeOutputsConfig, parseBaseSafeOutputConfig)
safe_outputs_app_config.go 341 GitHubAppConfig, token mint/invalidation steps, resolveActivationToken, config merging
safe_outputs_messages_config.go 153 parseMessagesConfig, parseMentionsConfig, serializeMessagesConfig, setStringFromMap

No behavioral changes. All public APIs preserved. Also fixes a stale logger reference — parseMentionsConfig was using safeOutputsConfigLog from the original file instead of the local safeOutputMessagesLog.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go k/gh�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD ode-gyp-bin/node-json go (http block)

If you need me to access, download, or install something from one of these locations, you can either:


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits March 12, 2026 06:42
- Extract GitHub App config/token generation into safe_outputs_app_config.go (341 lines)
- Extract messages/mentions parsing into safe_outputs_messages_config.go (153 lines)
- Reduce safe_outputs_config.go from 1077 to 605 lines
- All public APIs preserved; no behavioral changes
- Build and tests pass

Co-authored-by: pelikhan <[email protected]>
Replace safeOutputsConfigLog with safeOutputMessagesLog in parseMentionsConfig
to use the logger defined in the same file

Co-authored-by: pelikhan <[email protected]>
Copilot AI changed the title [WIP] [file-diet] Refactor pkg/workflow/safe_outputs_config.go into modules refactor: split safe_outputs_config.go into focused modules Mar 12, 2026
@github-actions
Copy link
Contributor

Hey @Copilot 👋 — great initiative splitting safe_outputs_config.go! The file had grown to 1085 lines in main, and the three-way split into safe_outputs_config.go (frontmatter), safe_outputs_app_config.go (GitHub App token lifecycle), and safe_outputs_messages_config.go (messages/mentions parsing) is a textbook application of the codebase's "Prefer many small files, group by functionality" principle. The bonus fix for the stale safeOutputsConfigLogsafeOutputMessagesLog logger reference is a nice catch too.

One thing worth addressing before this moves out of draft:

  • No test coverage signal in the diff — the three changed files are all production code; no test file was touched. Since this is a pure refactor with no behavioral changes, the existing safe_outputs_*_test.go suite should cover the split already. It would help reviewers to confirm that by running make test-unit and attaching a passing note, or by adding a small compile-time import test (similar to the existing safe_outputs_app_import_test.go and safe_outputs_import_test.go) for the two new files to assert the split is correct.

If you'd like a hand adding that coverage signal, here's a ready-to-use prompt:

In pkg/workflow/, two new files were introduced by PR #20639:
  - safe_outputs_app_config.go  (GitHubAppConfig, token mint/invalidation, resolveActivationToken)
  - safe_outputs_messages_config.go  (parseMessagesConfig, parseMentionsConfig, serializeMessagesConfig)

The existing test files safe_outputs_app_import_test.go and safe_outputs_import_test.go use blank-import compile checks.
Please:
1. Create pkg/workflow/safe_outputs_messages_config_import_test.go following the same pattern as safe_outputs_app_import_test.go — a package-level compile test that ensures the new file's exported symbols are reachable.
2. Run `make test-unit` and confirm all tests pass.
3. If any test references safeOutputsConfigLog (the old logger variable) instead of safeOutputMessagesLog, update those references.

Generated by Contribution Check ·

@pelikhan pelikhan marked this pull request as ready for review March 12, 2026 13:50
Copilot AI review requested due to automatic review settings March 12, 2026 13:50
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

Refactors pkg/workflow/safe_outputs_config.go by splitting it into smaller, domain-focused modules while preserving the existing APIs and behavior, and fixes a stale logger reference in mentions parsing.

Changes:

  • Extracted GitHub App configuration parsing/merging and token step generation into safe_outputs_app_config.go.
  • Extracted messages + mentions parsing/serialization helpers into safe_outputs_messages_config.go (including correcting the logger used in parseMentionsConfig).
  • Reduced safe_outputs_config.go to focus on frontmatter extraction / base config parsing and removed now-unneeded imports.

Reviewed changes

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

File Description
pkg/workflow/safe_outputs_config.go Removes app/messages domains from the monolith file; retains frontmatter extraction/base parsing and cleans up imports.
pkg/workflow/safe_outputs_app_config.go New module containing GitHubAppConfig, include-merge logic, and GitHub App token mint/invalidate step builders.
pkg/workflow/safe_outputs_messages_config.go New module containing messages parsing, mentions parsing (with corrected logger), and JSON serialization for env passing.

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

You can also share your feedback on Copilot code review. Take the survey.

@pelikhan pelikhan merged commit 5d0d1b1 into main Mar 12, 2026
113 checks passed
@pelikhan pelikhan deleted the copilot/refactor-safe-outputs-config branch March 12, 2026 14:00
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.

[file-diet] Refactor pkg/workflow/safe_outputs_config.go (1077 lines) into focused modules

3 participants