fix(strict-mode): reject explicit --as instead of silently overriding it

[sang-neo03]

Summary

ResolveAs checked strict mode before the --as flag, so --as bot under strict=user was silently rewritten to user. Reorder so explicit --as is returned as-is and
CheckStrictMode rejects the conflict (exit=2). Implicit paths (--as auto / unset) are still forced by strict mode.

Changes

• Reorder ResolveAs in internal/cmdutil/factory.go so explicit --as user|bot is returned before strict-mode forcing; strict mode only forces implicit paths.
• Update unit tests in internal/cmdutil/factory_test.go to cover explicit-vs-strict preservation and --as auto forcing.
• Update integration tests in cmd/root_integration_test.go for shortcut / service / api paths to assert the strict_mode validation envelope instead of silent override.

Test Plan

• Unit tests pass (go test ./internal/cmdutil/... -run TestResolveAs_StrictMode)
• Integration tests pass (go test ./cmd/... -run TestIntegration_StrictMode)
• Manual local verification: under strict=user, lark-cli im +chat-create --as bot --dry-run / lark-cli im chats get --as bot --dry-run / lark-cli api --as bot GET ... --dry-run all return exit=2 with strict_mode envelope; --as user / --as auto / unset still succeed. Symmetric checks under strict=bot also pass.

Related Issues

• Relates to lark-cli api --as bot ignored inside pipe (while read loop) #41

Summary by CodeRabbit

• Bug Fixes

  • Strict-mode no longer forces an identity before honoring an explicit selection; explicit identities are preserved and properly rejected with clear validation.
  • Auto-identity resolution now respects strict-mode constraints.

• Tests

  • Added unit tests for strict-mode resolution and rejection scenarios.
  • Updated integration tests to validate dry-run error output and strict-mode validation behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1727a7f2-5a62-4665-b7ec-e1f9c7d12f4f

📥 Commits

Reviewing files that changed from the base of the PR and between da4041b and 973f73d.

📒 Files selected for processing (1)

• cmd/root_integration_test.go

---

📝 Walkthrough

Walkthrough

ResolveAs now defers applying strict-mode forced identities until after explicit --as flag processing. Integration tests were updated to expect structured stderr output.ErrorEnvelope validation failures. Unit tests were added to cover explicit --as preservation and --as auto resolution under strict-mode.

Changes

Cohort / File(s)	Summary
Strict-Mode Integration Tests cmd/root_integration_test.go	Refactored dry-run assertions to validate structured stderr output.ErrorEnvelope with OK:false, ExitValidation, and Error.Type=="strict_mode". Removed stdout JSON dry-run parsing and added tests that supply explicitly disallowed --as values and expect strict-mode error envelopes containing the attempted Identity and message.
Factory Identity Resolution internal/cmdutil/factory.go	Reordered ResolveAs to return explicit --as values unchanged and apply strict-mode forced identity only when --as is auto or unset. Removed prior behavior that forced identity before considering explicit --as.
Factory Unit Tests internal/cmdutil/factory_test.go	Added three tests: two assert explicit --as (bot/user) are preserved and later rejected by CheckStrictMode; one verifies explicit --as auto under user-strict resolves to core.AsUser instead of following bot auto-detection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudge the flags and bounce with glee,
First read the --as, then strict-mode sees me.
Envelopes catch what I cannot be,
Tests hop forward, logic set free. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.33% 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 concisely summarizes the main change: rejecting explicit --as flags instead of silently overriding them in strict-mode, which aligns with the core objective of the pull request.
Description check	✅ Passed	The pull request description includes all required template sections: Summary (explaining the motivation), Changes (listing modifications across three files), Test Plan (with checkboxes and specific test commands), and Related Issues. It provides comprehensive context for the changes.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/strict-mode-explicit-as-reject

---

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]

🧹 Nitpick comments (1)

cmd/root_integration_test.go (1)

152-164: Remove unused parseDryRunJSON function.

This helper function at lines 152–164 is no longer called anywhere in the file after the refactor to envelope assertions. Removing it will eliminate the dead code and its strings usage (though strings is still used elsewhere in the file).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/root_integration_test.go` around lines 152 - 164, Remove the unused
helper function parseDryRunJSON from the test file: delete the parseDryRunJSON
function declaration (including its t.Helper call, prefix check, JSON unmarshal,
and return) since it's no longer referenced after switching to envelope
assertions; ensure any import only used by that function (if now unused) is
cleaned up by the importer (e.g., remove strings if it becomes unused).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmd/root_integration_test.go`:
- Around line 152-164: Remove the unused helper function parseDryRunJSON from
the test file: delete the parseDryRunJSON function declaration (including its
t.Helper call, prefix check, JSON unmarshal, and return) since it's no longer
referenced after switching to envelope assertions; ensure any import only used
by that function (if now unused) is cleaned up by the importer (e.g., remove
strings if it becomes unused).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a502268c-90f6-4a7f-b9a5-80043dc90c7f

📥 Commits

Reviewing files that changed from the base of the PR and between 1e2144e and da4041b.

📒 Files selected for processing (3)

• cmd/root_integration_test.go
• internal/cmdutil/factory.go
• internal/cmdutil/factory_test.go

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/strict-mode-explicit-as-reject -y -g

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.96%. Comparing base (1e2144e) to head (973f73d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #673   +/-   ##
=======================================
  Coverage   62.96%   62.96%           
=======================================
  Files         436      436           
  Lines       38604    38605    +1     
=======================================
+ Hits        24306    24307    +1     
  Misses      12150    12150           
  Partials     2148     2148           

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