Silent error discards on os.Setenv / os.Chdir in pkg/cli (10 prod sites)

[github-actions]

Summary

Sergo Run 14 identified 10 production sites in pkg/cli/ that silently discard error returns from os.Setenv (4 sites) and os.Chdir (6 sites). Unlike other _ = patterns in the codebase, these do NOT meet any of the four established defensibility criteria (callee logs internally, documented best-effort, error-cleanup with propagated primary error, init-time API contract). A failure produces no diagnostic and silently corrupts environment state or working directory state, leading to confusing downstream errors elsewhere.

Severity: High

Three of the four os.Setenv sites handle secrets (Copilot token, system PAT, generic API key) in user-facing prompt flows. A silent failure presents the user with a success message (✓ Valid fine-grained Copilot token received) while leaving the env var unset; downstream calls then read empty/stale values and surface as opaque auth failures unrelated to the actual storage failure.

The os.Chdir sites restore the working directory after a temporary Chdir into a checkout or trial sandbox. A silent restore failure leaves the process in a directory that may have been removed by sibling cleanup logic, breaking all subsequent file operations in the same process without explanation.

Affected Sites

os.Setenv discards (4)

File	Line	Discard Style	Stored Value
pkg/cli/add_interactive_orchestrator.go	73	implicit (os.Setenv(...) as statement)	auto-detected GHES host (GH_HOST)
pkg/cli/engine_secrets.go	329	_ = os.Setenv(...)	Copilot fine-grained token
pkg/cli/engine_secrets.go	377	_ = os.Setenv(...)	system PAT
pkg/cli/engine_secrets.go	430	_ = os.Setenv(...)	generic engine API key

os.Chdir discards (6)

File	Line	Discard Style
pkg/cli/deploy_command.go	131	_ = os.Chdir(originalDir) in defer closure
pkg/cli/update_command.go	220	_ = os.Chdir(originalDir) in defer closure
pkg/cli/trial_repository.go	211	implicit defer os.Chdir(originalDir)
pkg/cli/trial_repository.go	230	implicit defer os.Chdir(tempDir)
pkg/cli/trial_repository.go	540	implicit defer os.Chdir(originalDir)
pkg/cli/trial_helpers.go	326	implicit defer os.Chdir(originalDir)

The four implicit-defer sites are worse than _ = because Go's errcheck and similar linters may miss the discarded return entirely.

Defensibility Test (Run 14 Framework)

For reference, the following _ = patterns in pkg/cli/ ARE defensible and require no change:

• compile_pipeline.go:454,455,469,507,526,569 and compile_file_operations.go:134,135 (_ = saveActionCache(...), _ = updateGitAttributes(...), _ = purgeOrphanedLockFiles(...), _ = purgeInvalidFiles(...)) — callees log internally before returning the error (see compile_infrastructure.go:25-29,55-58).
• mcp_tools_privileged.go:594 (_ = req.Session.NotifyProgress(...)) — best-effort with explicit doc comment at line 585-588.
• pr_helpers.go:47,52,57,63 (_ = switchBranch(currentBranch, verbose)) — error-cleanup paths with a primary error already being propagated to the caller.
• git.go:304,307,310,311 (_ = exec.Command("git", ...).Run()) — best-effort batch git staging in stageWorkflowChanges().
• *_command.go _ = cmd.Flags().MarkHidden(...) / MarkDeprecated(...) / MarkFlagRequired(...) — init-time cobra API; error only when flag doesn't exist (caller bug, not runtime).
• run.go:47,94 _ = cancelCmd.Run() // Ignore errors for individual cancellations — annotated.
• glob_validation.go:141 _ = v.scan.Next() // eat '-'; return value not needed — annotated.

The 10 sites in this issue meet NONE of these criteria.

Recommendation

Option A (preferred): introduce logging helpers in a small cliutil package

// pkg/cli/cliutil/syscalls.go
package cliutil

import (
    "os"
    "github.com/githubnext/gh-aw/pkg/logger"
)

// SetenvOrWarn calls os.Setenv and logs a warning if it fails.
// Use this when failure should not abort the user-facing flow but must leave a diagnostic.
func SetenvOrWarn(log *logger.Logger, name, value string) {
    if err := os.Setenv(name, value); err != nil {
        log.Printf("failed to set env var %s: %v", name, err)
    }
}

// ChdirOrWarn calls os.Chdir and logs a warning if it fails. Intended for defer cleanup.
func ChdirOrWarn(log *logger.Logger, dir string) {
    if err := os.Chdir(dir); err != nil {
        log.Printf("failed to chdir back to %s: %v", dir, err)
    }
}

Then replace each site with the helper call. The implicit-defer sites become:

// Before:
defer os.Chdir(originalDir)
// After:
defer cliutil.ChdirOrWarn(trialRepositoryLog, originalDir)

Option B: inline log-on-failure

// Before:
_ = os.Setenv(req.Name, token)
// After:
if err := os.Setenv(req.Name, token); err != nil {
    engineSecretsLog.Printf("failed to set env var %s: %v", req.Name, err)
}

Validation

• No _ = os.Setenv(...) or _ = os.Chdir(...) remains in pkg/cli/ prod files
• No defer os.Chdir(...) implicit-discard pattern remains in pkg/cli/ prod files
• go test ./pkg/cli/... passes
• make lint passes (errcheck, if enabled)
• Manual smoke: trigger an os.Chdir failure (e.g., delete originalDir mid-call) and confirm the warning surfaces in verbose log

Estimated Effort: Small (~10 sites, mechanical edit + one helper file)

Context

Discovered by Sergo Run 14 (strategy: open-issue-reverify-plus-silent-syscall-discard-audit). The cached Run 13 open issues #aw_sgcn1 and #aw_sgcn2 were both verified resolved this run, freeing analysis budget for this new pattern audit.

References:

• §26142388027

Generated by 🤖 Sergo - Serena Go Expert · ● 20.8M · ◷

• expires on May 27, 2026, 5:14 AM UTC

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · ● 286.5K · ◷