feat(cli): silence proxy warning in non-interactive invocations

[zhengzhijiej-tech]

Problem

transport.WarnIfProxied prints a one-time proxy notice to stderr whenever an HTTP(S)_PROXY env var is detected (and not disabled via LARK_CLI_NO_PROXY). For human interactive use this is a useful security hint.

But agent / CI / piped callers parse the CLI's stdout as JSON and frequently merge streams with 2>&1. In that case the stray stderr warning corrupts the parsed payload, breaking machine consumers.

Change

Gate the warning on interactivity — i.e. whether stdin is a TTY:

• WarnIfProxied(w io.Writer) → WarnIfProxied(w io.Writer, interactive bool); returns early when interactive == false.
• Callers in internal/cmdutil/factory_default.go pass f.IOStreams.IsTerminal.

Human interactive sessions still get the security notice; machine-consumed output stays clean. Passing interactive=false does not consume the once guard, so a later interactive call can still warn.

Scope

Split out of the lark-sheets refactor PR as an independent, generic CLI/agent-UX change. Only touches the proxy-warning path:

• internal/transport/warn.go
• internal/transport/warn_test.go (adds a non-interactive-silence case)
• internal/cmdutil/factory_default.go (2 call sites)

Verification

go build ./... and go test ./internal/transport/... ./internal/cmdutil/... pass.

Summary by CodeRabbit

• Bug Fixes
  • Proxy warnings are now suppressed in non-interactive environments (scripts, CI pipelines), while remaining visible during normal terminal usage.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request adds an interactive parameter to the WarnIfProxied function to suppress proxy warnings in non-interactive contexts like CI environments. The factory now passes terminal status from IO streams, and tests verify both suppression and preservation of existing warning behaviors.

Changes

Proxy Warning Interactivity Control

Layer / File(s)	Summary
WarnIfProxied function signature and suppression logic internal/transport/warn.go	The WarnIfProxied function adds an interactive bool parameter and returns early to suppress output when interactive is false, preventing proxy warning noise in CI and non-terminal contexts.
HTTP and Lark client initialization with terminal awareness internal/cmdutil/factory_default.go	Both HTTP client and Lark client setup paths now pass f.IOStreams.IsTerminal to WarnIfProxied, enabling warnings only when stderr is a terminal.
Test updates and new non-interactive suppression case internal/transport/warn_test.go	All existing test calls are updated to pass interactive=true, and a new test TestWarnIfProxied_SilentWhenNonInteractive verifies warnings are suppressed when interactive=false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• larksuite/cli#1213: Both PRs touch internal/transport/warn.go's WarnIfProxied API and its call sites; this PR adds the interactive bool parameter while the retrieved PR introduced the function into internal/transport.

Suggested labels

size/L

Suggested reviewers

• liangshuo-1
• albertnusouo

Poem

🐰 A proxy warning rings so clear,
But in the machines, no ears to hear!
Terminal-aware, the CLI now knows—
When interactive's false, the warning goes,
Keeping CI logs clean and bright,
Just the way we like it, right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures the main change: suppressing proxy warnings in non-interactive CLI invocations, which is the primary objective of this PR.
Description check	✅ Passed	The description comprehensively covers problem statement, implementation details, scope, and verification steps, aligning well with the template requirements.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.22%. Comparing base (24ce3ec) to head (f4a9094).

Files with missing lines	Patch %	Lines
internal/cmdutil/factory_default.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1241   +/-   ##
=======================================
  Coverage   69.22%   69.22%           
=======================================
  Files         639      639           
  Lines       59798    59800    +2     
=======================================
+ Hits        41395    41397    +2     
  Misses      15058    15058           
  Partials     3345     3345           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

🧹 Nitpick comments (1)

internal/transport/warn_test.go (1)

80-95: ⚡ Quick win

Consider covering the once-guard-preservation contract.

This test verifies suppression when interactive=false, but the documented behavior that a non-interactive call does not consume proxyWarningOnce (so a subsequent interactive call still warns) is untested. A small follow-up assertion would lock in that contract.

💚 Suggested additional assertion

	// A non-interactive call must not consume the once guard:
	// a later interactive call should still warn.
	var buf2 bytes.Buffer
	WarnIfProxied(&buf2, true)
	if buf2.Len() == 0 {
		t.Error("expected warning on interactive call after a suppressed non-interactive call")
	}

Note: to make this assertion reliable, this test should also call unsetProxyPluginEnv(t) / resetProxyPluginState() like the sibling tests, since the interactive path reaches proxyPluginStatus().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/transport/warn_test.go` around lines 80 - 95, Add an assertion to
TestWarnIfProxied_SilentWhenNonInteractive that verifies the once-guard is not
consumed by the non-interactive path: after the existing non-interactive call to
WarnIfProxied(&buf, false) set up the same test env cleanup (call
unsetProxyPluginEnv(t) and resetProxyPluginState()) and then call WarnIfProxied
with interactive=true into a new buffer (e.g., buf2) and assert buf2.Len() > 0
so an interactive call still emits the warning; reference the proxyWarningOnce
guard and proxyPluginStatus indirectly by exercising WarnIfProxied to ensure the
contract is preserved.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/transport/warn_test.go`:
- Around line 80-95: Add an assertion to
TestWarnIfProxied_SilentWhenNonInteractive that verifies the once-guard is not
consumed by the non-interactive path: after the existing non-interactive call to
WarnIfProxied(&buf, false) set up the same test env cleanup (call
unsetProxyPluginEnv(t) and resetProxyPluginState()) and then call WarnIfProxied
with interactive=true into a new buffer (e.g., buf2) and assert buf2.Len() > 0
so an interactive call still emits the warning; reference the proxyWarningOnce
guard and proxyPluginStatus indirectly by exercising WarnIfProxied to ensure the
contract is preserved.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 36e35052-5268-4c09-b09e-e537e960ff32

📥 Commits

Reviewing files that changed from the base of the PR and between 24ce3ec and f4a9094.

📒 Files selected for processing (3)

• internal/cmdutil/factory_default.go
• internal/transport/warn.go
• internal/transport/warn_test.go

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@f4a9094c5cfd46b94aedd2e6dc9dc55339727205

🧩 Skill update

npx skills add zhengzhijiej-tech/cli#feat/cli-proxy-warn-noninteractive -y -g