Update site webmanifest with PWA icons and binary-extensions#23
Update site webmanifest with PWA icons and binary-extensions#23vdimarco wants to merge 5 commits into
Conversation
- Added android-chrome-192x192.png and android-chrome-512x512.png icons - Added site.webmanifest with app details and icons Enables progressive web app functionality with appropriate icons and manifest configuration. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors a sim executor test to actually execute and await the workflow, adds several dependencies to Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Case
participant Exec as Executor
participant Parent as Parent Workflow
participant Child as Child Workflow
rect rgb(230,245,255)
Note over Test,Exec: New test flow executes and awaits the workflow
Test->>Exec: create Executor and call execute('test-workflow-id')
Exec->>Parent: start parent workflow
Parent->>Child: invoke child workflow
Child--xParent: error thrown by child
Parent--xExec: propagate error to executor
Exec-->>Test: returned failed result / rejected
end
rect rgb(250,240,245)
Note right of Test: Test awaits and asserts that error propagated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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. Comment |
Amp-Thread-ID: https://ampcode.com/threads/T-34f5f20c-8c44-4cb0-9bfa-e1298e626716 Co-authored-by: Amp <amp@ampcode.com>
vdimarco
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Amp-Thread-ID: https://ampcode.com/threads/T-34f5f20c-8c44-4cb0-9bfa-e1298e626716 Co-authored-by: Amp <amp@ampcode.com>
![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
♻️ Duplicate comments (1)
apps/sim/package.json (1)
76-76: Dependency conflict previously flagged.A previous review identified that adding
binary-extensions@3.1.0creates a version conflict with the transitive dependencyis-binary-path@2.1.0, which requiresbinary-extensions@^2.0.0. This major version bump may introduce breaking changes and could result in both v3.1.0 and v2.3.0 being present in node_modules.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/sim/public/android-chrome-192x192.pngis excluded by!**/*.pngapps/sim/public/android-chrome-512x512.pngis excluded by!**/*.pngbun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/sim/executor/index.test.ts(1 hunks)apps/sim/package.json(1 hunks)apps/sim/providers/gatewayz/index.ts(4 hunks)apps/sim/providers/utils.ts(1 hunks)apps/sim/public/site.webmanifest(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/sim/providers/gatewayz/index.ts (2)
apps/sim/providers/types.ts (2)
ProviderRequest(136-168)ProviderResponse(74-100)apps/sim/executor/types.ts (1)
StreamingExecution(191-194)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test and Build
🔇 Additional comments (6)
apps/sim/public/site.webmanifest (1)
1-11: LGTM!The PWA manifest structure is valid and properly configured with icon references, theme colors, and standalone display mode.
apps/sim/providers/gatewayz/index.ts (3)
3-7: LGTM!The import reorganization improves code organization by consolidating type imports and clarifying dependencies from utilities. No functional changes.
63-65: LGTM!Function signature formatting is clear and readable.
174-174: LGTM!Consistent arrow function parameter formatting.
apps/sim/providers/utils.ts (1)
7-7: LGTM!Import reordering improves code organization consistency with no functional changes.
apps/sim/executor/index.test.ts (1)
1400-1455: Test skip is due to mock infrastructure limitations, not code structure issues.The test is correctly written and ready to run, but it's skipped because the current mock setup cannot simulate failing child workflows. The
WorkflowBlockHandlermock always returns success regardless of theworkflowIdparameter; there's no mechanism to configure specific workflow IDs to fail. The test expectsresult.errorto contain 'Error in child workflow', but the mocks don't support this scenario.To enable this test, the mock infrastructure would need enhancement to:
- Accept configuration for per-workflowId failure scenarios, or
- Conditionally fail the
'failing-child-workflow'ID insetupAllMocks()Consider tracking this as a technical debt item and updating the skip reason with a comment referencing the mock infrastructure limitation.
| "name": "", | ||
| "short_name": "", |
There was a problem hiding this comment.
Empty PWA name fields will result in poor installation experience.
The name and short_name fields are currently empty strings. When users attempt to install this PWA, browsers will typically display the URL or "Untitled" instead of a meaningful app name. Please update these fields with appropriate values before promoting PWA installation.
🤖 Prompt for AI Agents
In apps/sim/public/site.webmanifest around lines 2 to 3, the "name" and
"short_name" fields are empty which leads to poor PWA install UX; update them to
meaningful values by setting "name" to the full application display name (e.g.,
"Simulator App" or the project's proper title) and "short_name" to a shorter
label (<=12 characters) suitable for launcher/shortcut displays, ensuring they
are localized if needed and valid non-empty strings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/docs/package.json (1)
35-36: Clarify the override pattern for React type versions.The current configuration broadens
devDependenciesto^19(lines 35-36) but then addsoverrideswith^19.1.13and^19.1.9(lines 42-45). Since the overrides still use caret ranges, they permit versions above 19.1.13/19.1.9, which may not achieve the intended pinning.Recommendations:
- If exact pinning is needed, use
"@types/react": "19.1.13"(without^) in overrides- If caret ranges are acceptable, clarify the intent—typically overrides are used to force a minimum version across the dependency tree
- Consider whether the
overridesblock is necessary given thedevDependenciesalready specify^19Also applies to: 42-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/docs/app/[lang]/layout.tsx(1 hunks)apps/docs/app/layout.tsx(1 hunks)apps/docs/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test and Build
🔇 Additional comments (3)
apps/docs/app/layout.tsx (1)
1-1: LGTM: Consistent type annotation style.Using
React.ReactNodeinline is a valid approach and aligns with the similar change inapps/docs/app/[lang]/layout.tsx, improving consistency across layout files.apps/docs/app/[lang]/layout.tsx (1)
49-49: LGTM: Consistent type annotation style.The switch to
React.ReactNodematches the change inapps/docs/app/layout.tsx, maintaining consistent typing across layout components.apps/docs/package.json (1)
26-28: I'll verify the latest version information and security status for these packages through web searches.Based on my verification:
remark-gfm 4.0.1 is the latest version (last published 8 months ago), remark-mdx 3.1.1 is the latest version (last published 24 days ago), and remark 15.0.1 is current. The npm package remark-gfm was scanned for known vulnerabilities and missing license, and no issues were found. The npm audit also returned no vulnerabilities for any of these packages. remark-gfm version 4 is compatible with remark version 15+, and all three packages maintain active development within the unified collective.
1 participant
Summary
Changes
{"name":"",
📎 Task: https://www.terragonlabs.com/task/3bc64db6-061e-444e-ad7d-70bc648d3172
Summary by CodeRabbit
Tests
Chores
Style