Skip to content

refactor(envvars): consolidate agent env value access - #1757

Merged
evandance merged 2 commits into
mainfrom
feat/agent-name-env
Jul 6, 2026
Merged

refactor(envvars): consolidate agent env value access#1757
evandance merged 2 commits into
mainfrom
feat/agent-name-env

Conversation

@liangshuo-1

@liangshuo-1 liangshuo-1 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Tidy up how agent-related env values are read: the read + sanitize now live in one place (internal/envvars), shared by AgentTrace and a new AgentName accessor.

envvars.AgentName() reads LARKSUITE_CLI_AGENT_NAME so business code can identify the agent driving the CLI. Value is sanitised (trim, reject control chars, 128-byte cap) and returns "" when unset or invalid. Local-only; not sent to the server.
Move agent-trace reading from cmdutil.AgentTraceValue into envvars.AgentTrace(), symmetric with AgentName and sharing one sanitizeSingleLine. secheader.go now only assembles headers (BaseSecurityHeaders calls envvars.AgentTrace()); the duplicated sanitize and its os/unicode imports are gone. The X-Agent-Trace header and its behaviour are unchanged.
@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 6, 2026
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR relocates agent-trace sanitization logic from internal/cmdutil/secheader.go into a new internal/envvars package, introducing AgentName() and AgentTrace() functions backed by a shared sanitizeSingleLine helper. BaseSecurityHeaders() now calls envvars.AgentTrace(), and the local AgentTraceValue() helper along with its tests are removed.

Changes

Env var sanitization relocation

Layer / File(s) Summary
New envvars constants and sanitized readers
internal/envvars/envvars.go, internal/envvars/read.go
Adds CliAgentName constant and new AgentName()/AgentTrace() functions using a shared sanitizeSingleLine helper that trims whitespace, enforces max length, and rejects control characters.
Tests for envvars readers
internal/envvars/read_test.go
New test file covering empty/unset env vars, trimming, control-character rejection, and length boundaries for both functions.
Wire BaseSecurityHeaders to envvars.AgentTrace
internal/cmdutil/secheader.go, internal/cmdutil/secheader_test.go
Removes local AgentTraceValue() and unused imports; BaseSecurityHeaders() now sources HeaderAgentTrace from envvars.AgentTrace(); corresponding obsolete unit tests are removed and test section renamed.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant BaseSecurityHeaders
  participant envvars

  Caller->>BaseSecurityHeaders: build headers
  BaseSecurityHeaders->>envvars: AgentTrace()
  envvars-->>BaseSecurityHeaders: sanitized value or ""
  BaseSecurityHeaders-->>Caller: headers with HeaderAgentTrace (if non-empty)
Loading

Possibly related PRs

  • larksuite/cli#1158: Implements the original HeaderAgentTrace derivation from an agent-trace env var, whose sanitization logic this PR relocates into envvars.AgentTrace().
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is only one sentence and omits the required Summary, Changes, Test Plan, and Related Issues sections. Rewrite it to match the template with a brief summary, bullet list of changes, test plan, and related issues.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title is concise and accurately summarizes the refactor around consolidating agent env value access.
✨ 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/agent-name-env

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

Comment @coderabbitai help to get the list of available commands.

@liangshuo-1 liangshuo-1 changed the title feat(envvars): add LARKSUITE_CLI_AGENT_NAME accessor refactor(envvars): consolidate agent env value access Jul 6, 2026
@github-actions github-actions Bot added size/L Large or sensitive change across domains or core paths and removed size/M Single-domain feat or fix with limited business impact labels Jul 6, 2026
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1757   +/-   ##
=======================================
  Coverage   74.42%   74.42%           
=======================================
  Files         854      855    +1     
  Lines       88457    88481   +24     
=======================================
+ Hits        65832    65856   +24     
+ Misses      17556    17555    -1     
- Partials     5069     5070    +1     

☔ 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

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/agent-name-env -y -g

@evandance
evandance merged commit 91d785f into main Jul 6, 2026
102 of 112 checks passed
@evandance
evandance deleted the feat/agent-name-env branch July 6, 2026 13:17
This was referenced Jul 6, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Jul 29, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants