v0.8.40: browser agent improvements, permission requests - #7876
Open
waleedlatif1 wants to merge 8 commits into
Open
waleedlatif1 wants to merge 8 commits into
waleedlatif1 wants to merge 8 commits into
Conversation
…7868) * chore(db): drop retired usage columns and compatibility scaffolding * fix(db): forward force flags for local and dev schema pushes
…ty (#7872) * fix(browser): preserve click targets and bound screenshot capture * fix(browser): preserve observations and support native form controls * fix(browser): share snapshot text budget with inline fragments
* feat(access-requests): request and review permission access * fix(access-requests): reuse resource states and harden review lifecycle * fix(access-requests): recheck rollout and retain public models
* fix(workspace): hydrate access policy before rendering chat * fix(workspace): update layout test setup for access prefetch
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
github-actions
Bot
added
the
requires-mothership-merge
|
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+480
to
+485
| const capture = (async () => { | ||
| try { | ||
| return await contents.capturePage(undefined, { stayHidden: true }) | ||
| } finally { | ||
| pendingScreenshotCaptures.delete(contents) | ||
| } |
Contributor
There was a problem hiding this comment.
When capturePage exceeds the five-second timeout, the caller fails but the native promise remains pending and keeps this tab in pendingScreenshotCaptures. If the native operation never settles, every later screenshot on the tab is rejected as already pending, permanently disabling screenshots until the tab is replaced. The affected tab needs a recovery or retirement path after timeout.
Knowledge Base Used:
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 222 files
Confidence score: 2/5
apps/desktop/src/main/browser-agent/cdp.tscan leavependingScreenshotCapturespopulated indefinitely when the five-second timeout rejects before the nativecapturePagepromise settles, causing stale state or resource growth; ensure timeout handling performs independent cleanup or cancellation.apps/sim/lib/permission-access-requests/policy.tsandapps/sim/lib/permission-groups/application/read-user-config.tscan expose requestable or blocking access-control UI for targets that deployment enforcement excludes or when self-hosted Access Control is disabled, creating user-facing policy inconsistencies; resolve both paths through the shared deployment regime and environment allowlist.apps/sim/lib/api/contracts/access-requests.tsaccepts arbitrary strings in enum-backed policy snapshots, weakening contract validation and allowing malformed changes through; validatebeforeandafterwith the canonical read schema.apps/sim/components/access-requests/my-access-requests.tsxhides allowed and unavailable catalog entries by filtering torequestable, whileapps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/toolbar/toolbar.tsxopens a new request flow for already-pending integration requests; preserve all discovery states and route pending rows to the existing request.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/permission-access-requests/policy.ts">
<violation number="1" location="apps/sim/lib/permission-access-requests/policy.ts:58">
P2: When a deployment-wide integration allowlist excludes a catalog target, this raw group config can mark it allowed or requestable even though enforcement rejects it. Merge the environment allowlist into the resolved group before evaluating targets and building deltas.</violation>
</file>
<file name="apps/sim/lib/api/contracts/access-requests.ts">
<violation number="1" location="apps/sim/lib/api/contracts/access-requests.ts:214">
P2: Policy-change snapshots accept arbitrary strings for enum-backed permission fields because this contract reuses one generic list schema for every `configKey`. Validate `before` and `after` with the canonical read schema for the selected permission field before exposing or persisting previews and history.</violation>
</file>
<file name="apps/sim/components/access-requests/my-access-requests.tsx">
<violation number="1" location="apps/sim/components/access-requests/my-access-requests.tsx:50">
P2: When Browse access loads, `state: 'requestable'` filters out every allowed and unavailable entry, making the corresponding rendering branches unreachable. Remove this filter so the catalog displays all discovery states.</violation>
</file>
<file name="apps/sim/lib/permission-groups/application/read-user-config.ts">
<violation number="1" location="apps/sim/lib/permission-groups/application/read-user-config.ts:32">
P2: When self-hosted Access Control is disabled, this still resolves an entitled organization’s group and can block the UI despite server enforcement being inactive. Gate resolution through the shared deployment-regime check before returning the group config.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/toolbar/toolbar.tsx">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/toolbar/toolbar.tsx:533">
P2: When an integration request is already pending, clicking its restricted toolbar row opens the creation modal instead of the existing request. Discover integration targets and route `pendingRequestId` to the workspace access-requests page; only open the creation modal when no pending request exists.</violation>
</file>
<file name="apps/desktop/src/main/browser-agent/cdp.ts">
<violation number="1" location="apps/desktop/src/main/browser-agent/cdp.ts:482">
P1: When the five-second timer wins, `captureViewportImage` rejects while the `capturePage` promise continues running; its `finally` is the only cleanup for `pendingScreenshotCaptures`. If the native promise never settles, later screenshots remain rejected forever. Retire or recreate the tab, or otherwise clear the guard through a timeout recovery path before returning the timeout error.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| }) | ||
| const capture = (async () => { | ||
| try { | ||
| return await contents.capturePage(undefined, { stayHidden: true }) |
Contributor
There was a problem hiding this comment.
P1: When the five-second timer wins, captureViewportImage rejects while the capturePage promise continues running; its finally is the only cleanup for pendingScreenshotCaptures. If the native promise never settles, later screenshots remain rejected forever. Retire or recreate the tab, or otherwise clear the guard through a timeout recovery path before returning the timeout error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/browser-agent/cdp.ts, line 482:
<comment>When the five-second timer wins, `captureViewportImage` rejects while the `capturePage` promise continues running; its `finally` is the only cleanup for `pendingScreenshotCaptures`. If the native promise never settles, later screenshots remain rejected forever. Retire or recreate the tab, or otherwise clear the guard through a timeout recovery path before returning the timeout error.</comment>
<file context>
@@ -456,8 +452,48 @@ function sameScreenshotViewport(
+ })
+ const capture = (async () => {
+ try {
+ return await contents.capturePage(undefined, { stayHidden: true })
+ } finally {
+ pendingScreenshotCaptures.delete(contents)
</file context>
| (isHosted | ||
| ? await isOrganizationOnEnterprisePlan(context.organizationId, 'return-false', executor) | ||
| : isAccessControlEnabled) | ||
| const group = !entitled |
Contributor
There was a problem hiding this comment.
P2: When a deployment-wide integration allowlist excludes a catalog target, this raw group config can mark it allowed or requestable even though enforcement rejects it. Merge the environment allowlist into the resolved group before evaluating targets and building deltas.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/permission-access-requests/policy.ts, line 58:
<comment>When a deployment-wide integration allowlist excludes a catalog target, this raw group config can mark it allowed or requestable even though enforcement rejects it. Merge the environment allowlist into the resolved group before evaluating targets and building deltas.</comment>
<file context>
@@ -0,0 +1,122 @@
+ (isHosted
+ ? await isOrganizationOnEnterprisePlan(context.organizationId, 'return-false', executor)
+ : isAccessControlEnabled)
+ const group = !entitled
+ ? null
+ : context.workspaceId
</file context>
| export type AccessRequestResponse = z.output<typeof accessRequestResponseSchema> | ||
|
|
||
| export const accessRequestPolicyValueSchema = storedAccessRequestPolicyValueSchema | ||
| export const accessRequestPolicyChangeSchema = storedAccessRequestPolicyChangeSchema |
Contributor
There was a problem hiding this comment.
P2: Policy-change snapshots accept arbitrary strings for enum-backed permission fields because this contract reuses one generic list schema for every configKey. Validate before and after with the canonical read schema for the selected permission field before exposing or persisting previews and history.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/api/contracts/access-requests.ts, line 214:
<comment>Policy-change snapshots accept arbitrary strings for enum-backed permission fields because this contract reuses one generic list schema for every `configKey`. Validate `before` and `after` with the canonical read schema for the selected permission field before exposing or persisting previews and history.</comment>
<file context>
@@ -0,0 +1,312 @@
+export type AccessRequestResponse = z.output<typeof accessRequestResponseSchema>
+
+export const accessRequestPolicyValueSchema = storedAccessRequestPolicyValueSchema
+export const accessRequestPolicyChangeSchema = storedAccessRequestPolicyChangeSchema
+export type AccessRequestPolicyChange = z.output<typeof accessRequestPolicyChangeSchema>
+export const accessRequestDecisionSchema = storedAccessRequestDecisionSchema
</file context>
| { | ||
| ...scope, | ||
| search: debouncedSearch, | ||
| state: 'requestable', |
Contributor
There was a problem hiding this comment.
P2: When Browse access loads, state: 'requestable' filters out every allowed and unavailable entry, making the corresponding rendering branches unreachable. Remove this filter so the catalog displays all discovery states.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/components/access-requests/my-access-requests.tsx, line 50:
<comment>When Browse access loads, `state: 'requestable'` filters out every allowed and unavailable entry, making the corresponding rendering branches unreachable. Remove this filter so the catalog displays all discovery states.</comment>
<file context>
@@ -0,0 +1,204 @@
+ {
+ ...scope,
+ search: debouncedSearch,
+ state: 'requestable',
+ limit: ACCESS_REQUEST_PAGE_SIZE,
+ offset,
</file context>
| isOrganizationOnEnterprisePlan(organizationId, 'throw'), | ||
| ]) | ||
| : [false, false] | ||
| const resolved = |
Contributor
There was a problem hiding this comment.
P2: When self-hosted Access Control is disabled, this still resolves an entitled organization’s group and can block the UI despite server enforcement being inactive. Gate resolution through the shared deployment-regime check before returning the group config.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/permission-groups/application/read-user-config.ts, line 32:
<comment>When self-hosted Access Control is disabled, this still resolves an entitled organization’s group and can block the UI despite server enforcement being inactive. Gate resolution through the shared deployment-regime check before returning the group config.</comment>
<file context>
@@ -0,0 +1,46 @@
+ isOrganizationOnEnterprisePlan(organizationId, 'throw'),
+ ])
+ : [false, false]
+ const resolved =
+ organizationId && entitled
+ ? await resolveWorkspaceGroup(principal.userId, organizationId, context.workspaceId)
</file context>
| .sort((a, b) => a.name.localeCompare(b.name)) | ||
| }, [customBlocksData, currentWorkflowId, fallbackIconUrl]) | ||
|
|
||
| const handleRequestItemClick = useCallback( |
Contributor
There was a problem hiding this comment.
P2: When an integration request is already pending, clicking its restricted toolbar row opens the creation modal instead of the existing request. Discover integration targets and route pendingRequestId to the workspace access-requests page; only open the creation modal when no pending request exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/toolbar/toolbar.tsx, line 533:
<comment>When an integration request is already pending, clicking its restricted toolbar row opens the creation modal instead of the existing request. Discover integration targets and route `pendingRequestId` to the workspace access-requests page; only open the creation modal when no pending request exists.</comment>
<file context>
@@ -502,6 +530,13 @@ export const Toolbar = memo(
.sort((a, b) => a.name.localeCompare(b.name))
}, [customBlocksData, currentWorkflowId, fallbackIconUrl])
+ const handleRequestItemClick = useCallback(
+ (type: string) => {
+ if (accessRequestsEnabled) setRequestedBlockType(type)
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.