fix(desktop): scope cron job calls by profile - #47429
Conversation
|
Related: #42654 (also scopes Desktop cron calls to the active profile). Same goal, different mechanism — #42654 threads an explicit |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for covering the full cron helper surface.
Problems
profileScoped()routes Electron to a backend, but local and per-profile-remote paths retain the original URL (apps/desktop/electron/connection-config.ts:199-233). The cron API defaults list requests toalland creates todefault(hermes_cli/web_server.py:10268-10270,10376-10378), so the change does not establish the requested profile selection for those paths.- The added test currently fails Desktop type-checking: CI reports
TS2353for{ paused: true }inapps/desktop/src/hermes.test.ts:84;CronJobUpdatesexposesenabled, notpaused(apps/desktop/src/types/hermes.ts:580-586).
Suggested changes
- Keep backend routing, but add an endpoint-profile mechanism that handles local pools while preserving global/per-profile remote behavior; test those three routing shapes.
- Use a valid update field and adapt the test to current
getCronJobs()request shape.
Automated hermes-sweeper review.
| @@ -523,18 +523,21 @@ export function testMessagingPlatform(platformId: string): Promise<MessagingPlat | |||
|
|
|||
| export function getCronJobs(): Promise<CronJob[]> { | |||
| return window.hermesDesktop.api<CronJob[]>({ | |||
There was a problem hiding this comment.
request.profile selects an Electron backend but does not set the cron endpoint's profile argument on local/per-profile-remote paths. Those paths retain this URL, whose list endpoint defaults to all and whose create endpoint defaults to default; scope the endpoint contract as well.
| await getCronJob('job 1') | ||
| await getCronJobRuns('job 1', 7) | ||
| await createCronJob({ name: 'Daily', prompt: 'say hi', schedule: '0 9 * * *' }) | ||
| await updateCronJob('job 1', { paused: true }) |
There was a problem hiding this comment.
CronJobUpdates has enabled?: boolean, not paused; the PR's Desktop TypeScript check fails with TS2353 here. Use a valid update field such as { enabled: false }.
384d09b to
d97bdb9
Compare
|
Thanks @necoweb3 — you were the FIRST to submit the profileScoped() mechanism for the cron helpers (June 16, ahead of the June 21 and July submissions of the same fix). The identical approach landed on main via #67493, and a contract test covering all 9 helpers landed via #67602 — the sweep only surfaced your PR afterward, so belated but real credit: you had the right fix first. The sweeper-flagged local-pool gap (routing alone doesn't filter the list endpoint) is addressed in #67615. Closing as landed-on-main. |
Summary
This scopes Desktop cron job REST helpers to the active API profile.
Why
The backend cron endpoints are profile-aware, but the Desktop helpers did not pass the active API profile. In a multi-profile Desktop session, the Cron page could show or mutate jobs against the default/launch profile instead of the selected profile.
The most visible case is creating or editing a cron job while another profile is active: the UI appears to operate on that profile, but the request can land in the default profile's cron store.
Changes
profileScoped()to cron job list/detail/run-history helpers.profileScoped()to create/update/pause/resume/trigger/delete helpers.Tests