Closed
Add rationale comments to production osgetenvlibrary suppressions#44096
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add rationale comments to 45 bare nolint suppressions
Add rationale comments to production osgetenvlibrary suppressions
Jul 7, 2026
Contributor
🤖 Triage Assessment
Summary: Adds rationale comments to 45 nolint suppressions across 27 files. Draft. No logic change, +45/−45. Next: Convert to ready-for-review when reviewed by maintainer.
|
This was referenced Jul 7, 2026
pelikhan
marked this pull request as ready for review
July 8, 2026 01:50
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves auditability of production //nolint:osgetenvlibrary suppressions by adding short, callsite-local rationales explaining why direct process environment reads are intentional boundaries (host/token discovery, runtime detection, CI/Codespaces checks, secret sourcing, explicit overrides).
Changes:
- Added one-line rationale comments to
os.Getenv/os.LookupEnvcallsites that retain//nolint:osgetenvlibrarysuppressions across the CLI and supporting packages. - Kept behavior unchanged (comment-only updates) while making future reviews of env-bound reads easier.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/process_env_lookup.go | Adds rationale for process env lookup helper suppression. |
| pkg/parser/github.go | Adds rationales for GitHub host/token env override reads. |
| pkg/logger/logger.go | Adds rationales for DEBUG/ACTIONS_RUNNER_DEBUG/DEBUG_COLORS reads. |
| pkg/github/label_objective_mapping.go | Adds rationale for env-injected objective mapping JSON override. |
| pkg/envutil/envutil.go | Adds rationale for centralized env read in integer parsing helper. |
| pkg/constants/constants.go | Adds rationale for workflow directory env override. |
| pkg/console/accessibility.go | Adds rationales for accessibility-related terminal env flags. |
| pkg/cli/update_check.go | Adds rationale for MCP-mode env marker check. |
| pkg/cli/shell_completion.go | Adds rationales for shell detection and Homebrew prefix env reads. |
| pkg/cli/secrets.go | Adds rationale for env-first secret availability detection. |
| pkg/cli/secret_set_command.go | Adds rationale for --from-env secret value resolution. |
| pkg/cli/mcp_validation.go | Adds rationales for env checks during MCP server secret validation. |
| pkg/cli/mcp_tools_privileged.go | Adds rationale for inheriting GITHUB_REPOSITORY into privileged tool args. |
| pkg/cli/mcp_server_command.go | Adds rationale for reading GITHUB_ACTOR during actor validation. |
| pkg/cli/mcp_repository.go | Adds rationale for preferring GITHUB_REPOSITORY in repo resolution. |
| pkg/cli/logs_command.go | Adds rationale for MCP-container fast-path GITHUB_REPOSITORY matching. |
| pkg/cli/interactive.go | Adds rationale for interactive-mode guards using test/CI env markers. |
| pkg/cli/init.go | Adds rationales for GHES detection using GitHub/gh CLI env hints. |
| pkg/cli/import_url_fetcher.go | Adds rationale for GH_HOST env usage in import auth host resolution. |
| pkg/cli/engine_secrets.go | Adds rationales for env-based secret discovery and export paths. |
| pkg/cli/compile_update_check.go | Adds rationale for env-based update-check opt-out. |
| pkg/cli/compile_orchestrator.go | Adds rationale for GH_HOST gating of auto-detection behavior. |
| pkg/cli/codespace.go | Adds rationale for Codespaces runtime env marker detection. |
| pkg/cli/ci.go | Adds rationale for CI env marker detection loop. |
| pkg/cli/add_wizard_command.go | Adds rationale for CI env check gating interactive wizard use. |
| pkg/cli/add_interactive_workflow.go | Adds rationale for Codespaces UX gating via env marker. |
| pkg/cli/add_interactive_orchestrator.go | Adds rationales for interactive guards and GH_HOST gating of auto-detection. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 27/27 changed files
- Comments generated: 1
- Review effort level: Low
Comment on lines
76
to
+77
| // Check for MCP_SERVER environment variable that could be set by the MCP server | ||
| return os.Getenv("GH_AW_MCP_SERVER") != "" //nolint:osgetenvlibrary | ||
| return os.Getenv("GH_AW_MCP_SERVER") != "" //nolint:osgetenvlibrary // MCP mode detection intentionally uses a dedicated process-env marker. |
github-actions
Bot
deleted the
copilot/deep-report-add-rationale-comments
branch
July 15, 2026 02:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Production code had 45
//nolint:osgetenvlibrarysuppressions with no explanation, which made it hard to distinguish intentional environment-bound reads from stale exemptions. This updates each surviving production suppression to carry a short rationale at the callsite.What changed
osgetenvlibrarysuppression across CLI, parser, console, logger, workflow, constants, and GitHub integration code.Auditability
os.Getenv/os.LookupEnvusage rather than documented elsewhere.Scope
Example of the updated pattern: