Support guest script runs in Desktop#1096
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78c7509a4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '/runs/:jobSection?/:jobID?/:jobDeviceID?', | ||
| '/run/:jobID?/:jobDeviceID?', |
There was a problem hiding this comment.
Adding '/run/:jobID?/:jobDeviceID?' here routes URLs like /run/123 into ScriptingRouter, but that router only redirects the exact /run path and has no '/run/:... handler, so these links render an empty scripting panel instead of reaching a valid page. Because the outer route now matches first, the global fallback redirect is bypassed; add an internal redirect for '/run/:jobID/:jobDeviceID?' (for example to /runs/job/...) or remove this matcher.
Useful? React with 👍 / 👎.
| await dispatch.jobs.delete({ jobId: job.id, fileId: file.id }) | ||
| }} | ||
| /> | ||
| {file && ( |
There was a problem hiding this comment.
This guard hides delete whenever file is falsy, but on /runs/job/:jobID routes fileID is not present so script (and therefore file) stays undefined even for jobs owned by the active account. That means opening a normal run from the Runs tab removes the Delete Run action unexpectedly; this should be keyed off ownership or scriptFileRef and use that id for the delete redirect context.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1589219e7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Route | ||
| path={[ | ||
| '/scripts', | ||
| '/runs/:jobSection?/:jobID?/:jobDeviceID?', |
There was a problem hiding this comment.
Keep the /runs/:fileID route parameter name available instead of replacing it with :jobSection: existing navigation still sends users to /runs/${fileID} from ScriptPage (the “View all runs” button), and JobsPage reads fileID from params when calling dispatch.jobs.fetch({ fileID }). With this change, that param is no longer populated, so script-scoped history links now load the global runs list instead of the selected script’s runs.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a81f33988d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Panel layout={layout}> | ||
| <JobsPage /> | ||
| </Panel> | ||
| <Route path="/runs"> |
There was a problem hiding this comment.
fileID in runs route matching
Changing this route to "/runs" drops the :fileID param from the active match, but JobsPage still reads fileID from useParams and ScriptPage still links to /runs/<fileID> for “View all runs”. In React Router v5, useParams reads params from the current matched route context, so /runs/<fileID> now behaves like an unfiltered runs page (and shows the split-panel layout) instead of a file-scoped runs list. Keep :fileID? on the matched route or explicitly forward that param into JobsPage.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00be3bf9be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Panel layout={layout}> | ||
| <JobsPage /> | ||
| </Panel> | ||
| <DynamicPanel primary={<JobsPage />} secondary={<></>} root="/runs" layout={layout} /> |
There was a problem hiding this comment.
/runs/:fileID routes
Passing secondary={<></>} here makes DynamicPanel treat /runs/:fileID as a detail route in single-panel mode, so it renders the empty fragment instead of JobsPage. This regresses the existing “View all runs” flow from ScriptPage (history.push(/runs/${fileID})), which now lands on a blank panel for filtered run URLs. Remove the truthy secondary prop (or gate it to real detail routes) so /runs/:fileID continues to show the runs list.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2db850121
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| attributes={attributes} | ||
| {...{ required, jobs, columnWidths, fetching }} | ||
| activeJobId={jobMatch?.params.jobID} | ||
| jobOnlyRoute |
There was a problem hiding this comment.
JobsPage now always sets jobOnlyRoute, so every run click navigates to /runs/job/:jobID even for scripts the user owns. On that route JobDetailPage has no fileID, which makes script/file undefined and forces scriptArguments to [], so the run detail silently drops the entire Arguments section for normal runs. This is a functional regression from the previous /script/:fileID/:jobID flow where argument metadata was available.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0143262451
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!job && jobID && !jobsFetching) { | ||
| dispatch.jobs.fetchSingle({ jobId: jobID }) |
There was a problem hiding this comment.
This effect now always calls dispatch.jobs.fetchSingle({ jobId: jobID }) whenever job is missing, but /script/:fileID/latest is a valid route used by jobs.save/jobs.run redirects and latest is not an actual job ID. In that case fetchSingle never resolves a job, jobs.fetching toggles back to false, and the same branch runs again, causing repeated requests and leaving deep-linked/reloaded latest-run pages stuck instead of falling back to file-based job loading.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61b4ac48d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [panelWidth, setPanelWidth] = usePanelWidth() | ||
| const location = useLocation() | ||
| const routeKey = location.pathname.match(REGEX_FIRST_PATH)?.[0].substring(1) || '' | ||
| const secondaryMinWidth = getPanelWidthDefault(routeKey) |
There was a problem hiding this comment.
Using getPanelWidthDefault(routeKey) directly as the secondary panel minimum makes DoublePanel unusable on routes that are not listed in PANEL_WIDTH_DEFAULTS (for example /partner-stats in PartnerStatsPage), because the fallback is 9999. On those routes this drives getMaxWidth() negative and forces impossible widths, so the split layout collapses and cannot be resized correctly.
Useful? React with 👍 / 👎.
- DoublePanel: clamp unknown-route secondary min width to MIN_WIDTH - JobDetailPage: prevent infinite fetchSingle loop on missing/deleted jobs - JobDetailPage: show loading state while fetch is in flight - JobDetailPage: show "File Deleted" copy for missing files - ScriptingRouter: drop redundant inner Switch in /runs/job route - JobListItem: remove unreachable null check - Drop unused file.shortDesc / file.longDesc from job query and types Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b90a4f2357
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (fileID) { | ||
| if (!fetching && file) dispatch.jobs.fetchByFileIds({ fileIds: [file.id] }) | ||
| return | ||
| } |
There was a problem hiding this comment.
This branch returns as soon as fileID exists, even when file is undefined, so fetchSingle is never attempted for /script/:fileID/:jobID URLs that reference guest/private scripts not present in the local file list. In that case the page can only render Job not found despite having a valid jobID, which breaks direct links/bookmarks that still use the script-scoped route.
Useful? React with 👍 / 👎.
| if (attemptedJobIds.current.has(jobID)) return | ||
| attemptedJobIds.current.add(jobID) |
There was a problem hiding this comment.
Once a jobID is added to attemptedJobIds, subsequent renders will never call fetchSingle for that ID again, even if the selected job later disappears from cache (for example after account/context changes or list refresh churn). That leaves the detail view stuck in a permanent not-found state for the same URL because recovery fetches are blocked by this guard.
Useful? React with 👍 / 👎.
- Fall through to fetchSingle when fileID is set but the file is not in our local cache, so guest/private scripts and bookmarked /script/:fileID/:jobID links resolve via job-id lookup. - Reset attempt tracking on account change so users can retry the same URL under the new context. - Track file- and job-scoped fetches independently and gate the file branch on jobsFetching to avoid duplicate fetches. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Use the URL's fileID to choose between /script and /runs links so clicking a device on a /runs/job/:jobID page navigates within /runs instead of jumping to the script page. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Changes