Skip to content

test(im): drop --yes from chats link e2e (not high-risk-write) - #700

Merged
liangshuo-1 merged 1 commit into
mainfrom
fix/im-chats-link-yes-flag
Apr 28, 2026
Merged

test(im): drop --yes from chats link e2e (not high-risk-write)#700
liangshuo-1 merged 1 commit into
mainfrom
fix/im-chats-link-yes-flag

Conversation

@liangshuo-1

@liangshuo-1 liangshuo-1 commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the failing TestIM_ChatsLinkWorkflow/get_chat_share_link_as_bot on main (and on every open PR) by removing a stray Yes: true from the e2e Request.

Root cause

im chats link is a generic service method whose metadata does not carry risk: "high-risk-write", so cmd/service/service.go never registers the --yes flag on it. Once #633 landed and added Yes: true to this test, the e2e runner started appending --yes to every invocation; cobra rejects it before the request is dispatched:

Error: unknown flag: --yes

That tanks AssertExitCode(t, 0), then AssertStdoutStatus, then the share_link should not be empty check. CI run on main confirming the regression: https://github.com/larksuite/cli/actions/runs/25056050086

Why drop the flag instead of marking the API as high-risk-write

Generating a chat share link with a configurable validity period is non-destructive — it doesn't delete data, doesn't alter membership, and the link can be invalidated. The other tests touched in #633 (calendar event delete, sheets filter remove, task delete, etc.) genuinely flipped to high-risk-write at the metadata layer; im.chats.link did not. So the Yes: true here is leftover from the bulk pass, not a missing metadata flip.

If product later decides share-link creation should require confirmation, the right move is to flip the metadata risk and re-add Yes: true together — not to keep a flag that the binary doesn't accept.

Test plan

  • go build ./...
  • go vet ./tests/cli_e2e/im/...
  • e2e-live job green on this PR (the only assertion this PR touches)

Summary by CodeRabbit

Release Notes

No user-facing changes in this release. This update includes internal test infrastructure modifications only.

`im chats link` is registered as a regular service method (no
`risk: high-risk-write` annotation), so the framework does not register
the `--yes` flag on it. Setting `Yes: true` on the e2e Request makes
the runner append `--yes`, which cobra rejects with `unknown flag:
--yes` before the request is ever issued — the rest of the assertions
then fall through with empty stdout.

The flag was added in #633 alongside the risk-tiering rollout that
covered other workflows that genuinely flipped to high-risk-write.
For chats link the API call (creating a chat share link with a
configurable validity period) is not destructive and was never
re-classified, so the line is just leftover from that pass. Drop it
to restore the e2e green; if we ever decide to gate share-link
creation behind confirmation we can re-add it together with the
metadata flip.

Change-Id: Ieb094407a7f0fa18cd130a9d80c7146274b5ecc7
@github-actions github-actions Bot added the size/S Low-risk docs, CI, test, or chore only changes label Apr 28, 2026
@coderabbitai

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 36ffe9ff-a06c-463b-84f9-9226b381cfc1

📥 Commits

Reviewing files that changed from the base of the PR and between 7eb0ba3 and d7ac927.

📒 Files selected for processing (1)
  • tests/cli_e2e/im/chat_workflow_test.go
💤 Files with no reviewable changes (1)
  • tests/cli_e2e/im/chat_workflow_test.go

📝 Walkthrough

Walkthrough

The e2e test for im chats link command is updated to remove an explicit confirmation flag from the command request payload. The test continues to verify the same command behavior without this field set.

Changes

Cohort / File(s) Summary
Test Configuration
tests/cli_e2e/im/chat_workflow_test.go
Removed the explicit Yes: true confirmation flag from the chat link command request payload.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A test hops along, confirms what it knows,
One flag removed, the workflow still goes,
No yes or no, just the path taking flight—
The chat link dances, still shining so bright! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: removing the --yes flag from the im chats link e2e test with justification (not high-risk-write).
Description check ✅ Passed The description comprehensively covers all template sections: a clear summary of the fix, detailed root cause analysis, rationale for the approach, and a test plan with specific steps.
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.

✏️ 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/im-chats-link-yes-flag

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 and usage tips.

@codecov

codecov Bot commented Apr 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.80%. Comparing base (7eb0ba3) to head (d7ac927).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #700   +/-   ##
=======================================
  Coverage   63.80%   63.80%           
=======================================
  Files         500      500           
  Lines       43531    43531           
=======================================
  Hits        27773    27773           
  Misses      13317    13317           
  Partials     2441     2441           

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

@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/im-chats-link-yes-flag -y -g

@liangshuo-1
liangshuo-1 merged commit 4422265 into main Apr 28, 2026
20 checks passed
@liangshuo-1
liangshuo-1 deleted the fix/im-chats-link-yes-flag branch April 28, 2026 14:06
HomyeeKing pushed a commit to HomyeeKing/cli that referenced this pull request May 6, 2026
…uite#700)

`im chats link` is registered as a regular service method (no
`risk: high-risk-write` annotation), so the framework does not register
the `--yes` flag on it. Setting `Yes: true` on the e2e Request makes
the runner append `--yes`, which cobra rejects with `unknown flag:
--yes` before the request is ever issued — the rest of the assertions
then fall through with empty stdout.

The flag was added in larksuite#633 alongside the risk-tiering rollout that
covered other workflows that genuinely flipped to high-risk-write.
For chats link the API call (creating a chat share link with a
configurable validity period) is not destructive and was never
re-classified, so the line is just leftover from that pass. Drop it
to restore the e2e green; if we ever decide to gate share-link
creation behind confirmation we can re-add it together with the
metadata flip.

Change-Id: Ieb094407a7f0fa18cd130a9d80c7146274b5ecc7
tuxedomm pushed a commit to zhumiaoxin/cli that referenced this pull request Jun 6, 2026
…uite#700)

`im chats link` is registered as a regular service method (no
`risk: high-risk-write` annotation), so the framework does not register
the `--yes` flag on it. Setting `Yes: true` on the e2e Request makes
the runner append `--yes`, which cobra rejects with `unknown flag:
--yes` before the request is ever issued — the rest of the assertions
then fall through with empty stdout.

The flag was added in larksuite#633 alongside the risk-tiering rollout that
covered other workflows that genuinely flipped to high-risk-write.
For chats link the API call (creating a chat share link with a
configurable validity period) is not destructive and was never
re-classified, so the line is just leftover from that pass. Drop it
to restore the e2e green; if we ever decide to gate share-link
creation behind confirmation we can re-add it together with the
metadata flip.

Change-Id: Ieb094407a7f0fa18cd130a9d80c7146274b5ecc7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Low-risk docs, CI, test, or chore only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants