Repository navigation
Workflow Copilot: show copilot button for screenshot browser - #4423
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6ef1daa in 50 seconds. Click for details.
- Reviewed
25lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. skyvern-frontend/src/routes/workflows/editor/Workspace.tsx:1537
- Draft comment:
The new flex wrapper for the WorkflowRunStream looks good for centering the stream. Please double-check that the aspect ratio styling is preserved as intended. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. skyvern-frontend/src/routes/workflows/editor/Workspace.tsx:1542
- Draft comment:
Consider adding an accessible aria-label to the new WorkflowCopilotButton and ensure it properly forwards the ref. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_mK63gc7hyVLdIdvA
You can customize 
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 6ef1daa in 1 minute and 40 seconds. Click for details.
- Reviewed
410lines of code in7files - Skipped
0files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. pyproject.toml:68
- Draft comment:
Removed zstandard dependency; ensure that HAR compression removal is intended. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. skyvern-frontend/src/routes/workflows/editor/Workspace.tsx:1534
- Draft comment:
Added WorkflowCopilotButton in the screenshot browser; verify its styling, import, and responsiveness. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify styling, import, and responsiveness, which falls under asking the author to double-check things. This violates the rules.
3. skyvern/forge/sdk/api/aws.py:16
- Draft comment:
Removed MIME type registration for .zst; confirm no other parts depend on the zstandard MIME type. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that no other parts depend on the zstandard MIME type. This falls under the rule of not asking the author to confirm their intention or to double-check things. Therefore, this comment should be removed.
4. skyvern/forge/sdk/api/real_azure.py:23
- Draft comment:
Removed MIME type registration for .zst in Azure API; ensure consistency with storage artifact handling. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking to ensure consistency with storage artifact handling, which is a form of asking the author to double-check something. This violates the rule against asking the author to ensure behavior is intended or to double-check things.
5. skyvern/forge/sdk/artifact/storage/s3.py:90
- Draft comment:
HAR artifact compression using zstandard has been removed; verify that storing HAR files uncompressed is the intended behavior. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. skyvern/forge/sdk/artifact/storage/s3.py:100
- Draft comment:
Updated _get_storage_class_for_org to require a 'bucket' parameter; ensure all call sites supply the correct bucket. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. skyvern/forge/sdk/artifact/storage/test_s3_storage.py:36
- Draft comment:
Tests related to HAR artifact compression (zstd) were removed; confirm that new tests adequately cover current behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that new tests adequately cover current behavior, which violates the rule against asking for confirmation or ensuring that changes are tested. It does not provide a specific suggestion or point out a specific issue with the code.
8. uv.lock:1
- Draft comment:
Removed zstandard package from uv.lock; check that dependency removal is consistent across the project. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to check for consistency across the project after removing a dependency. This falls under the rule of not asking the author to double-check things or ensure behavior is intended. It doesn't provide a specific suggestion or point out a specific issue.
9. uv.lock:6
- Draft comment:
It looks like the resolution-markers list is missing a closing bracket. Please ensure that the list is properly terminated (e.g., by adding a closing]on an appropriate line), so that the file has valid syntax. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_3QagpDdhmrJP0UYa
You can customize
…src/' <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a copilot button to the workflow editor interface for real-time workflow assistance and support during editing sessions. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Add `WorkflowCopilotButton` to the screenshot browser in `Workspace.tsx` for non-VNC streaming sessions. > > - **UI Enhancement**: > - Adds `WorkflowCopilotButton` to the screenshot browser section in `Workspace.tsx`. > - Button toggles `isCopilotOpen` state on click. > - **Behavior**: > - Button is displayed when `activeDebugSession` exists and `vnc_streaming_supported` is false. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 1e73da0a8c3262c7d91d65168c844e8c43a6c631. You can [customize](https://app.ellipsis.dev/Skyvern-AI/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
6ef1daa to
daedbfc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skyvern/forge/sdk/artifact/storage/s3.py (1)
416-467: Fix UUID-retry key construction to use a sanitized filename (avoid embedding paths in S3 keys).
The first attempt correctly usessanitized_filename = os.path.basename(filename), but the retry path usesfilenameagain, which can reintroduce path components (and surprising object keys).Proposed diff
# If upload fails, try again with UUID prefix if not uploaded_s3_uri: - uuid_prefixed_filename = f"{str(uuid.uuid4())}_{filename}" + uuid_prefixed_filename = f"{uuid.uuid4()}_{sanitized_filename}" s3_uri = f"s3://{bucket}/{settings.ENV}/{organization_id}/{todays_date}/{uuid_prefixed_filename}" fileObj.seek(0) # Reset file pointer uploaded_s3_uri = await self.async_client.upload_file_stream(s3_uri, fileObj, storage_class=sc, tags=tags)
🧹 Nitpick comments (3)
skyvern/forge/sdk/artifact/storage/test_s3_storage.py (1)
34-40: Silence Ruff ARG002 cleanly for the test override.
organization_id/bucketaren’t used inS3StorageForTests._get_storage_class_for_org, so Ruff will keep complaining. Consider underscore-prefixing the parameter names.Proposed diff
class S3StorageForTests(S3Storage): async def _get_tags_for_org(self, organization_id: str) -> dict[str, str]: return {"dummy": f"org-{organization_id}", "test": "jerry"} - async def _get_storage_class_for_org(self, organization_id: str, bucket: str) -> S3StorageClass: + async def _get_storage_class_for_org(self, _organization_id: str, _bucket: str) -> S3StorageClass: return S3StorageClass.ONEZONE_IAskyvern/forge/sdk/artifact/storage/s3.py (2)
103-115: (Optional) Decide whetherstore_artifactshould fail fast on upload errors.
Right nowAsyncAWSClient.upload_file()returnsNoneon failure, butstore_artifact()doesn’t check it—so callers may believe the artifact is stored when it isn’t. If the intended contract is “store must succeed”, consider raising when the upload returns falsy.Proposed diff
async def store_artifact(self, artifact: Artifact, data: bytes) -> None: sc = await self._get_storage_class_for_org(artifact.organization_id, self.bucket) tags = await self._get_tags_for_org(artifact.organization_id) LOG.debug( "Storing artifact", artifact_id=artifact.artifact_id, organization_id=artifact.organization_id, uri=artifact.uri, storage_class=sc, tags=tags, ) - await self.async_client.upload_file(artifact.uri, data, storage_class=sc, tags=tags) + uploaded = await self.async_client.upload_file(artifact.uri, data, storage_class=sc, tags=tags) + if not uploaded: + raise RuntimeError(f"Failed to upload artifact to S3: {artifact.uri}")
116-117: Silence Ruff ARG002 for_get_storage_class_for_org’s default implementation.
The base implementation doesn’t useorganization_id/bucket, so Ruff will flag it.Proposed diff
- async def _get_storage_class_for_org(self, organization_id: str, bucket: str) -> S3StorageClass: + async def _get_storage_class_for_org(self, _organization_id: str, _bucket: str) -> S3StorageClass: return S3StorageClass.STANDARD
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
pyproject.tomlskyvern-frontend/src/routes/workflows/editor/Workspace.tsxskyvern/forge/sdk/api/aws.pyskyvern/forge/sdk/api/real_azure.pyskyvern/forge/sdk/artifact/storage/s3.pyskyvern/forge/sdk/artifact/storage/test_s3_storage.py
💤 Files with no reviewable changes (2)
- pyproject.toml
- skyvern/forge/sdk/api/real_azure.py
🧰 Additional context used
📓 Path-based instructions (4)
skyvern-frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
skyvern-frontend/**/*.{ts,tsx,js,jsx}: Usenpm run lintandnpm run formatfor linting and formatting frontend code in skyvern-frontend/
Maintain line length of 120 characters for TypeScript/JavaScript code
Files:
skyvern-frontend/src/routes/workflows/editor/Workspace.tsx
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use async/await patterns for asynchronous operations
Files:
skyvern-frontend/src/routes/workflows/editor/Workspace.tsxskyvern/forge/sdk/artifact/storage/s3.pyskyvern/forge/sdk/api/aws.pyskyvern/forge/sdk/artifact/storage/test_s3_storage.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Useruff checkandruff formatfor linting and formatting Python code
Usemypyfor type checking Python code
Use type hints in Python code
Maintain line length of 120 characters for Python code
**/*.py: Use Python 3.11+ features and type hints in Python code
Follow PEP 8 with a line length of 100 characters in Python code
Use absolute imports for all modules in Python code
Document all public functions and classes with Google-style docstrings in Python code
Usesnake_casefor variables and functions,PascalCasefor classes in Python code
Prefer async/await over callbacks for asynchronous programming in Python
Useasynciofor concurrency in Python asynchronous code
Always handle exceptions in async code in Python
Use context managers for resource cleanup in Python
Use specific exception classes rather than generic exceptions in error handling
Include meaningful error messages in exceptions
Log errors with appropriate severity levels in Python
Never expose sensitive information in error messages
Optimize database queries in Python for performance
Use appropriate data structures in Python for performance optimization
Implement caching where beneficial in Python code
Validate all inputs in Python code
Use environment variables for configuration instead of hardcoding values
Files:
skyvern/forge/sdk/artifact/storage/s3.pyskyvern/forge/sdk/api/aws.pyskyvern/forge/sdk/artifact/storage/test_s3_storage.py
skyvern/forge/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
FastAPI-based REST API and WebSocket support should be located in
skyvern/forge/directory
Files:
skyvern/forge/sdk/artifact/storage/s3.pyskyvern/forge/sdk/api/aws.pyskyvern/forge/sdk/artifact/storage/test_s3_storage.py
🧠 Learnings (1)
📚 Learning: 2025-10-23T13:32:29.749Z
Learnt from: jomido
Repo: Skyvern-AI/skyvern PR: 3799
File: skyvern-frontend/src/components/BrowserStream.tsx:576-599
Timestamp: 2025-10-23T13:32:29.749Z
Learning: In Skyvern's controlled environment, the clipboard API (navigator.clipboard) is guaranteed to be available, so availability checks before clipboard operations are not necessary in skyvern-frontend/src/components/BrowserStream.tsx.
Applied to files:
skyvern-frontend/src/routes/workflows/editor/Workspace.tsx
🧬 Code graph analysis (3)
skyvern-frontend/src/routes/workflows/editor/Workspace.tsx (2)
skyvern-frontend/src/routes/workflows/workflowRun/WorkflowRunStream.tsx (1)
WorkflowRunStream(187-187)skyvern-frontend/src/routes/workflows/editor/WorkflowCopilotButton.tsx (1)
WorkflowCopilotButton(10-36)
skyvern/forge/sdk/artifact/storage/s3.py (3)
skyvern/forge/sdk/artifact/storage/test_s3_storage.py (1)
_get_storage_class_for_org(38-39)skyvern/forge/sdk/artifact/storage/base.py (1)
retrieve_artifact(95-96)skyvern/forge/sdk/artifact/models.py (1)
Artifact(56-87)
skyvern/forge/sdk/artifact/storage/test_s3_storage.py (2)
skyvern/forge/sdk/artifact/storage/s3.py (1)
_get_storage_class_for_org(116-117)skyvern/forge/sdk/api/aws.py (2)
bucket(501-502)S3StorageClass(24-33)
🪛 Ruff (0.14.10)
skyvern/forge/sdk/artifact/storage/s3.py
116-116: Unused method argument: organization_id
(ARG002)
116-116: Unused method argument: bucket
(ARG002)
119-119: Unused method argument: organization_id
(ARG002)
skyvern/forge/sdk/artifact/storage/test_s3_storage.py
38-38: Unused method argument: organization_id
(ARG002)
38-38: Unused method argument: bucket
(ARG002)
🔇 Additional comments (10)
skyvern-frontend/src/routes/workflows/editor/Workspace.tsx (2)
1537-1541: LGTM! Layout structure appropriately handles screenshot aspect ratio.The nested container structure with
items-center justify-centerandaspect-videoproperly centers and maintains the aspect ratio for the screenshot browser. This differs from the VNC browser's simpler approach, which is appropriate since screenshots may have different sizing requirements than the interactive VNC stream.
1542-1548: LGTM! Copilot button correctly added for screenshot browser.The footer implementation follows the same pattern as the VNC browser section and correctly implements the PR objective to show the copilot button for non-VNC streaming sessions. The button is properly wired to the existing
isCopilotOpenstate and uses the appropriate refs and handlers.skyvern/forge/sdk/artifact/storage/s3.py (7)
132-145: Bucket-aware storage class lookup wiring looks correct here.
Passingself.bucketinto_get_storage_class_for_org(...)aligns with the new signature; logging now usesartifact.uriconsistently.
146-161: Good: storage class is resolved against the screenshots bucket.
save_streaming_file()now calls_get_storage_class_for_org(…, AWS_S3_BUCKET_SCREENSHOTS), which matches the actual upload target.
166-183: Good: browser session uploads now resolve storage class against the browser-sessions bucket.
198-217: Good: browser profile uploads now resolve storage class against the browser-sessions bucket.
354-383: Good: uploads bucket is explicitly used for storage class resolution.
This matches thebase_uribucket and keeps the policy hook (_get_storage_class_for_org) bucket-aware.
483-497: Good: browser-session sync uses bucket-aware storage class resolution.
122-124: Verify there are no legacy ".zst" artifacts still referenced in storage/DB.Switching
retrieve_artifact()to downloadartifact.uriverbatim is clean, but if older runs stored HARs (or other artifacts) as*.zstkeys, those will become unreadable unless they've been migrated. Check the codebase and database schema for any remaining zstd-compressed artifact references and ensure a migration strategy exists if legacy artifacts are present.skyvern/forge/sdk/api/aws.py (1)
24-31: No action needed –GLACIER_IRis a valid AWS S3 StorageClass value.The addition of
GLACIER_IRto theS3StorageClassenum is correct. AWS accepts"GLACIER_IR"for put_object/upload_fileobj operations globally with no region restrictions.
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.
Important
Added copilot button for workflow assistance and removed zstandard compression for HAR files.
WorkflowCopilotButtontoWorkspace.tsxfor real-time workflow assistance.s3.py._get_storage_class_for_org()to includebucketparameter ins3.py.zstandardfrompyproject.tomlanduv.lock.GLACIER_IRtoS3StorageClassinaws.py.This description was created by
for 6ef1daa. You can customize this summary. It will automatically update as commits are pushed.
🤖 This PR adds a copilot button to the workflow editor's screenshot browser interface and removes zstandard compression functionality from the S3 storage system. The changes enhance user experience by providing AI assistance during workflow debugging while simplifying the storage architecture.
🔍 Detailed Analysis
Key Changes
WorkflowCopilotButtonto the screenshot browser section inWorkspace.tsxfor non-VNC streaming debug sessionszstandarddependencyGLACIER_IR(Glacier Instant Retrieval) storage class and updated storage class method signaturesTechnical Implementation
flowchart TD A[Debug Session Active] --> B{VNC Streaming Supported?} B -->|No| C[Show Screenshot Browser] C --> D[Display WorkflowRunStream] C --> E[Add Footer with Copilot Button] E --> F[Toggle Copilot Panel on Click] G[S3 Storage] --> H[Store Artifact] H --> I[No Compression Applied] I --> J[Direct Upload to S3] K[Retrieve Artifact] --> L[Direct Download from S3] L --> M[No Decompression Needed]Impact
Created with Palmier