Repository navigation
Plugin shot --scenario: screenshot a check scenario without touching the live plugin - #37090
Conversation
Findings
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: adding scenario-based screenshots that do not affect the live plugin. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description follows the repository template closely. It has an ELI5, What Changed, Why, Visual Proof (N/A with reason), Testing with checkboxes, Agent skill upstream boundary, Notes, and Checklist… Full details: Description check
-
Explanation
The description follows the repository template closely. It has an ELI5, What Changed, Why, Visual Proof (N/A with reason), Testing with checkboxes, Agent skill upstream boundary, Notes, and Checklist. The What Changed section covers the CLI, host RPC, renderer, refusals, result fields, and default output path. The Why section explains the safety constraint and the choice of a new RPC over a param on plugins.shot. The Linked Issue section reads 'N/A (maintainer change).' The template says a linked issue should always exist for outside contributors, and the maintainer special case says to attach pre-existing issues only. This is acceptable for a maintainer PR, but the AI Disclosure section is absent. The template says to skip that section only for StablyAI team members, so the author's membership status decides whether it is needed. The Review heading is also missing content. Testing lists unit and e2e coverage, and it names the platform as macOS only. The description does not mention Linux, Windows, or SSH testing, though the Notes section and checklist mention cross-platform impact was considered.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 23 functions across 28 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
44cffca1-f699-4708-9896-82c21d4d12f8
-
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
-
docs/reference/plugin-authoring.mdskill-guides/orca-cli/references/plugins.mdskill-guides/orca-plugin-authoring/references/cli.mdskill-guides/orca-plugin-authoring/references/workflow.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/plugin-dev-watch.test.tssrc/cli/handlers/plugin-dev-watch.tssrc/cli/handlers/plugin-shot-scenario.test.tssrc/cli/handlers/plugin-shot-scenario.tssrc/cli/handlers/plugin-view.tssrc/cli/plugin-dev-watch/plugin-dev-watch-events.tssrc/cli/plugin-dev-watch/plugin-dev-watch-shot.test.tssrc/cli/plugin-dev-watch/plugin-dev-watch-shot.tssrc/cli/specs/plugin-view.tssrc/main/plugins/view/plugin-shot-policy.tssrc/main/plugins/view/plugin-view-desktop.tssrc/main/plugins/view/plugin-view-replay-shot.test.tssrc/main/plugins/view/plugin-view-replay-shot.tssrc/main/runtime/rpc/methods/plugin-view.test.tssrc/main/runtime/rpc/methods/plugin-view.tssrc/renderer/src/App.tsxsrc/renderer/src/components/plugins/view/PluginShotReplayMount.tsxsrc/renderer/src/components/plugins/view/PluginShotReplaySurface.tsxsrc/renderer/src/components/plugins/view/plugin-view-replay.test.tsxsrc/renderer/src/components/plugins/view/plugin-view-replay.tssrc/renderer/src/components/plugins/view/plugin-view-request-handler.tssrc/renderer/src/components/right-sidebar/index.tsxsrc/renderer/src/components/right-sidebar/right-sidebar-width.tssrc/renderer/src/store/plugin-shot-replay.tssrc/shared/plugins/view/plugin-view-protocol.tssrc/shared/rpc-contract/plugins-params.tstests/e2e/plugin-shot.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
-
| const scenario = stringFlag(ctx, 'scenario') | ||
| if (scenario !== undefined) { | ||
| await scenarioShot(ctx, scenario) | ||
| return | ||
| } |
There was a problem hiding this comment.
A valueless --scenario silently takes a live shot.
stringFlag returns undefined for a boolean flag value. parseArgs stores true when --scenario has no value, or when the next token starts with --. Examples: orca plugin shot x --scenario and orca plugin shot x --scenario --theme dark.
In both cases scenario is undefined, so the handler skips scenarioShot. The command then opens and captures the live panel. The PR states that --scenario never falls back to a live shot. A valueless --scenario combined with --act also drives the live panel, so the --act conflict check is bypassed.
Check that the flag is present, and fail when it has no name.
🐛 Proposed fix
-
--- "a/src/cli/handlers/plugin-view.ts" +++ "b/src/cli/handlers/plugin-view.ts" @@ -263,11 +263,14 @@ ) }, 'plugin shot': async (ctx) => { - const scenario = stringFlag(ctx, 'scenario') - if (scenario !== undefined) { - await scenarioShot(ctx, scenario) - return - } + if (ctx.flags.has('scenario')) { + const scenario = stringFlag(ctx, 'scenario') + if (scenario === undefined) { + throw new RuntimeClientError('invalid_argument', '--scenario takes a scenario name.') + } + await scenarioShot(ctx, scenario) + return + } const pluginKey = requirePluginKey(ctx) const colorScheme = parseTheme(ctx) const out = stringFlag(ctx, 'out')
Committable suggestion
-
Review the replacement and indentation, then test before committing.Suggested changeconst scenario = stringFlag(ctx, 'scenario') if (scenario !== undefined) { await scenarioShot(ctx, scenario) return } if (ctx.flags.has('scenario')) { const scenario = stringFlag(ctx, 'scenario') if (scenario === undefined) { throw new RuntimeClientError('invalid_argument', '--scenario takes a scenario name.') } await scenarioShot(ctx, scenario) return }
…dev watch shot pendingRequests - shot <key|dir> --scenario <name> renders the panel in that check scenario through the check runner (a separate Node process on a mock host and mounted worker), sends the validated drawn tree to a new plugins.shotReplay, and captures it in a throwaway preview surface: no guest, no live host, no real storage or settings, no real worker, images as placeholders. Window hidden or minimized only; --act, --environment and bridged shells refused; an older host answers method_not_found (runtime_unsupported) rather than doing a live shot. - dev watch's shot event carries pendingRequests.
a754990 to
9b93a29
Compare
…dev watch shot pendingRequests (stablyai#37090) - shot <key|dir> --scenario <name> renders the panel in that check scenario through the check runner (a separate Node process on a mock host and mounted worker), sends the validated drawn tree to a new plugins.shotReplay, and captures it in a throwaway preview surface: no guest, no live host, no real storage or settings, no real worker, images as placeholders. Window hidden or minimized only; --act, --environment and bridged shells refused; an older host answers method_not_found (runtime_unsupported) rather than doing a live shot. - dev watch's shot event carries pendingRequests.
…dev watch shot pendingRequests (stablyai#37090) - shot <key|dir> --scenario <name> renders the panel in that check scenario through the check runner (a separate Node process on a mock host and mounted worker), sends the validated drawn tree to a new plugins.shotReplay, and captures it in a throwaway preview surface: no guest, no live host, no real storage or settings, no real worker, images as placeholders. Window hidden or minimized only; --act, --environment and bridged shells refused; an older host answers method_not_found (runtime_unsupported) rather than doing a live shot. - dev watch's shot event carries pendingRequests.
…dev watch shot pendingRequests (stablyai#37090) - shot <key|dir> --scenario <name> renders the panel in that check scenario through the check runner (a separate Node process on a mock host and mounted worker), sends the validated drawn tree to a new plugins.shotReplay, and captures it in a throwaway preview surface: no guest, no live host, no real storage or settings, no real worker, images as placeholders. Window hidden or minimized only; --act, --environment and bridged shells refused; an older host answers method_not_found (runtime_unsupported) rather than doing a live shot. - dev watch's shot event carries pendingRequests.
ELI5
Plugin authors can describe "scenarios" (empty, busy, error…) that
orca plugin checkrenders against fake data. Until now they could only read a text summary of those. Withorca plugin shot --scenario <name>they get a real screenshot of the panel in that state, and the user's actual plugin is never touched.What Changed
orca plugin shot <key|dir> --scenario <name> [--panel] [--theme] [--width] [--out] [--json]errors.plugins.shotReplayRPC.--actwith--scenario, remote hosts, bridged shells, installed plugins and HTML panels.replay: trueandscenario. The default file is.orca/shots/<key>-<panel>.<scenario>.png.dev watch: theshotevent now carriespendingRequests.Why
plugins.shot: an older host silently ignores unknown params, so it would have taken a LIVE shot instead. A new method answersmethod_not_foundon older hosts, and nothing is touched.Linked Issue
N/A (maintainer change).
Visual Proof
N/A: this produces PNGs and changes no app UI. The e2e test asserts that
shot --scenario countedon a scaffoldedworker-panelplugin is a non-blank PNG showing "Counter 41", in dark, at 320 px. It also assertsreplay: true, the live sidebar tab unchanged, the plugin's storage files unchanged, and the worker still idle.Testing
Unit:
dev watchpendingRequests.e2e: all plugin specs pass, 31/31, on a fresh build (macOS).
Gates:
pnpm lintand the quality gates pass.pnpm tcpasses in full on the rebased branch (main's unrelated test breakage was fixed in fix(test): correct structured session lifecycle imports #37088).I manually tested these changes locally
Automated tests added/updated, or explained why not below
Agent skill upstream boundary
Notes
checkdoes, and the help says so.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)