feat: simplify proxy plugin warning and gate on tty

[albertnusouo]

Summary

The proxy plugin warning printed two-to-three verbose lines (proxy address, disable instructions, and a custom-CA notice) on every command that built a network client, including non-interactive runs piped to files or consumed by agents. This collapses it to a single concise line and suppresses it entirely when stderr is not a terminal.

Changes

• Collapse the plugin warning in internal/transport/warn.go to one line: [lark-cli] [WARN] proxy plugin enabled: all requests are forced through proxy. (drops the address, the LARKSUITE_CLI_PROXY_ENABLE disable hint, and the custom-CA line); remove the now-unused envvars import
• Add a precomputed StderrIsTerminal field to IOStreams in internal/cmdutil/iostreams.go (computed once in NewIOStreams, mirroring the existing IsTerminal field)
• Gate both WarnIfProxied call sites in internal/cmdutil/factory_default.go on f.IOStreams.StderrIsTerminal so the notice is silent on pipes / CI / agent runs; introduce a warnIfProxied test seam (the real function is guarded by a sync.Once)
• Add internal/cmdutil/factory_proxy_warn_test.go: verify both client-init paths warn only when stderr is a terminal
• Update internal/transport/warn_test.go: assert the single-line message and credential non-leak; drop the obsolete custom-CA assertion test

Security note

The custom-CA / TLS-MITM line (...TLS to upstreams can be intercepted/inspected by this proxy) is intentionally removed, not overlooked. Proxy-plugin mode and a custom CA are both operator self-inflicted local configuration (not attacker-controllable), so this was a defense-in-depth awareness line rather than a vulnerability guard. Dropping it is a deliberate noise-vs-signal tradeoff; the remaining one-liner still states that all requests are forced through the proxy.

Test Plan

• go build ./... passed
• go vet ./internal/transport/ ./internal/cmdutil/ passed
• go test ./internal/transport/ ./internal/cmdutil/ passed (incl. new ProxyWarnGate tests: 4/4)
• manual verification (non-tty): LARKSUITE_CLI_PROXY_ENABLE=true LARKSUITE_CLI_PROXY_ADDRESS=http://127.0.0.1:8888 lark-cli auth login --domain docs 2>err.txt — 0 warning lines
• manual verification (tty via pty): ... script -q /dev/null lark-cli auth login --domain docs — exactly one proxy plugin enabled line
• skipped: local-eval / acceptance-reviewer — change is internal transport/IO plumbing with no shortcut or skill surface; covered by unit tests + manual e2e

Related Issues

N/A

Summary by CodeRabbit

• Improvements

  • Proxy warnings are now only shown in interactive terminal sessions to reduce log noise.
  • Proxy plugin warning messages are simplified and no longer reveal proxy configuration details.

• Tests

  • Added tests to ensure proxy warnings are emitted exactly in interactive terminals and suppressed for non-interactive/redirected output.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an exported IOStreams.StderrIsTerminal field computed from ErrOut, gates proxy warning emissions during cached HTTP and Lark client initialization on that field, simplifies the proxy-plugin warning to a single generic line, and updates/adds tests including a test seam to spy on proxy-warning invocations.

Changes

Proxy Warning Terminal Detection and Simplification

Layer / File(s)	Summary
Terminal detection field internal/cmdutil/iostreams.go	Adds exported IOStreams.StderrIsTerminal and sets it in NewIOStreams using term.IsTerminal when ErrOut is an *os.File.
Simplified proxy plugin warning internal/transport/warn.go	When proxy plugin mode is detected, WarnIfProxied now prints a single generic one-line warning; the previous address-/instruction-rich text and envvars usage were removed.
Terminal-gated warning emissions in client initialization internal/cmdutil/factory_default.go	Introduces an unexported warnIfProxied seam and calls it from cached HTTP and Lark client constructors only when IOStreams.StderrIsTerminal is true.
Updated transport warn tests internal/transport/warn_test.go	Tests updated to expect a single concise proxy-plugin warning line without proxy address leakage or disable-instruction text; related custom-CA assertions removed.
Factory-level proxy-warning gating tests internal/cmdutil/factory_proxy_warn_test.go	New tests install a spy for the package warnIfProxied seam and assert the warning fires only when StderrIsTerminal is true for both cached HTTP and Lark client initialization paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• larksuite/cli#1213: Related changes touching WarnIfProxied usage and tests.
• larksuite/cli#1181: Changes to proxy-plugin warning text and related logic.
• larksuite/cli#247: Prior edits that added proxy-warning side effects during client initialization.

Suggested labels

size/L

Suggested reviewers

• liangshuo-1

Poem

🐰 I checked the stderr fields tonight,
If it's a terminal, I'll shout the warning light,
Quiet when piped, loud when watched by you,
No leaked proxy secrets—just a single cue.
A little rabbit spy keeps the messages right.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat: simplify proxy plugin warning and gate on tty' directly and clearly summarizes the main changes: simplifying the warning message and gating it based on terminal detection.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
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.
Description check	✅ Passed	PR description comprehensively covers all template sections with detailed context about motivation, changes, testing approach, and security rationale.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/proxy-hint-opt

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@internal/cmdutil/factory_default.go`:
- Around line 105-107: Add unit tests for both cachedHttpClientFunc and
cachedLarkClientFunc that verify transport.WarnIfProxied is invoked only when
f.IOStreams.StderrIsTerminal() returns true: for each function, create two test
cases (StderrIsTerminal=true and false), inject a fake IOStreams with ErrOut
capturing writer and a stubbed StderrIsTerminal, replace or spy on
transport.WarnIfProxied to record invocations, call the client-init function,
and assert WarnIfProxied was called in the true case and not called in the false
case; ensure tests cover both client init paths.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 336f61fe-3e21-4973-9e28-ad1539ade8b0

📥 Commits

Reviewing files that changed from the base of the PR and between 0fbfe68 and 7ac5ee2.

📒 Files selected for processing (4)

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.35%. Comparing base (0fbfe68) to head (b74f5e2).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1448      +/-   ##
==========================================
+ Coverage   72.88%   73.35%   +0.46%     
==========================================
  Files         738      750      +12     
  Lines       69553    69250     -303     
==========================================
+ Hits        50695    50799     +104     
+ Misses      15043    14711     -332     
+ Partials     3815     3740      -75     

☔ View full report in Codecov by Harness.
📢 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.

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/proxy-hint-opt -y -g