Skip to content

fix(slides): declare screenshot scope#2044

Merged
ethan-zhx merged 1 commit into
mainfrom
codex/declare-slides-screenshot-scope
Jul 24, 2026
Merged

fix(slides): declare screenshot scope#2044
ethan-zhx merged 1 commit into
mainfrom
codex/declare-slides-screenshot-scope

Conversation

@ethan-zhx

@ethan-zhx ethan-zhx commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Declare the screenshot permission required by slides +screenshot so authentication preflight and generated scope guidance reflect the API requirement.

Changes

  • Add slides:presentation:screenshot to the shortcut's unconditional scopes.
  • Update the scope contract test for both user and bot identities, while preserving conditional wiki:node:read handling.
  • Remove the outdated allowlist guidance from the screenshot skill reference and retain the non-screenshot fallback guidance.

Test Plan

  • Unit tests pass (not run in this change; local repository instructions prohibit project test commands unless explicitly requested)
  • gofmt applied to the changed Go files
  • git diff --check passes
  • Static repository scan confirms no remaining outdated screenshot-scope guidance

Related Issues

  • None

Summary by CodeRabbit

  • Bug Fixes

    • Updated slide screenshot permission handling so the required screenshot access is declared and applied consistently.
    • Improved screenshot failure handling by falling back to XML read-back and structural or text-overlap checks.
  • Documentation

    • Clarified that slide validation can continue using non-screenshot checks when screenshot capture fails.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6402b82d-93b1-4b8f-99d8-fc578a16a7e6

📥 Commits

Reviewing files that changed from the base of the PR and between 5a54bc0 and 8f916dd.

📒 Files selected for processing (3)
  • shortcuts/slides/slides_screenshot.go
  • shortcuts/slides/slides_screenshot_test.go
  • skills/lark-slides/references/lark-slides-screenshot.md

📝 Walkthrough

Walkthrough

SlidesScreenshot now declares its screenshot OAuth scope, tests verify identity-specific scope behavior, and documentation describes non-screenshot validation fallback when screenshots fail.

Changes

Slides screenshot scope alignment

Layer / File(s) Summary
Screenshot scope declaration and identity tests
shortcuts/slides/slides_screenshot.go, shortcuts/slides/slides_screenshot_test.go
SlidesScreenshot declares slides:presentation:screenshot; tests verify user and bot preflight scopes and the combined declared scope list.
Screenshot fallback guidance
skills/lark-slides/references/lark-slides-screenshot.md
Screenshot failure guidance now describes XML read-back and related non-screenshot validation paths.

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

Possibly related PRs

  • larksuite/cli#1358: Introduces the SlidesScreenshot shortcut and its screenshot scope support.
  • larksuite/cli#1585: Updates overlapping SlidesScreenshot scope declarations and identity-scope tests.

Suggested reviewers: liangshuo-1, fangshuyu-768

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: declaring the slides screenshot scope.
Description check ✅ Passed The description follows the required template and includes summary, changes, test plan, and related issues.
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.
✨ 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 codex/declare-slides-screenshot-scope

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.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 24, 2026
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.13%. Comparing base (5a54bc0) to head (8f916dd).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2044   +/-   ##
=======================================
  Coverage   75.13%   75.13%           
=======================================
  Files         905      905           
  Lines       96130    96130           
=======================================
  Hits        72224    72224           
  Misses      18348    18348           
  Partials     5558     5558           

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

@ethan-zhx
ethan-zhx marked this pull request as ready for review July 24, 2026 04:55
@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#codex/declare-slides-screenshot-scope -y -g

@ethan-zhx
ethan-zhx merged commit 4807283 into main Jul 24, 2026
44 checks passed
@ethan-zhx
ethan-zhx deleted the codex/declare-slides-screenshot-scope branch July 24, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants