Surface errored system-initiated run-command jobs#5508
Conversation
…ng them A run-command job whose command finished with an error status resolved with the error in-band, so the queue marked it resolved and nothing alerted. A persistently failing system job (e.g. the daily OpenRouter model sync) was therefore silent. Add a required `alertOnError` flag to RunCommandArgs. When set, an error outcome (permission failure, invalid specifier, or a command that returns status:error) throws, so the queue marks the job rejected and reports it to Sentry. Interactive callers (bot-runner, the run-command HTTP endpoint, webhooks) pass false to keep a command error as a normal in-band result to hand back to the user; the system OpenRouter sync passes true. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e587e75 to
43265b5
Compare
Host Test Results 1 files ± 0 1 suites ±0 8m 58s ⏱️ - 2h 33m 34s Results for commit 1ebd683. ± Comparison against earlier commit 43265b5. Realm Server Test Results 1 files ±0 1 suites ±0 17m 2s ⏱️ -15s Results for commit 1ebd683. ± Comparison against earlier commit 43265b5. |
There was a problem hiding this comment.
Pull request overview
This PR introduces an alertOnError flag to the run-command task payload so system-initiated jobs (e.g. the OpenRouter model sync) can throw on command failures and be marked rejected (triggering existing queue/Sentry alerting), while interactive callers continue to receive in-band { status: 'error' } results.
Changes:
- Add required
alertOnError: booleantoRunCommandArgsand implement “throw vs in-band error” behavior in the task. - Set
alertOnError: truefor the system OpenRouter sync enqueue path; setfalsefor interactive/webhook/bot-runner callers. - Extend shared run-command task tests and re-export them through realm-server’s test module.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/tasks/run-command.ts | Adds alertOnError and throws on error outcomes for alerting callers. |
| packages/runtime-common/tests/run-command-task-shared-tests.ts | Adds shared tests covering alertOnError rejection behavior. |
| packages/realm-server/tests/run-command-task-test.ts | Adds realm-server test wrappers for the new shared test cases. |
| packages/realm-server/scripts/sync-openrouter-models.ts | Enqueues the system sync job with alertOnError: true. |
| packages/realm-server/handlers/handle-webhook-receiver.ts | Sets alertOnError: false for webhook-triggered commands. |
| packages/realm-server/handlers/handle-run-command.ts | Sets alertOnError: false for the interactive HTTP endpoint. |
| packages/bot-runner/lib/command-runner.ts | Sets alertOnError: false for interactive bot-runner enqueues. |
| packages/bot-runner/tests/command-runner-test.ts | Updates expected run-command payloads to include alertOnError: false. |
| packages/bot-runner/tests/bot-runner-test.ts | Updates expected run-command payloads to include alertOnError: false. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (alertOnError && result.status === 'error') { | ||
| let message = `${jobIdentity(jobInfo)} command ${command} failed in ${normalizedRealmURL}: ${result.error ?? 'unknown error'}`; | ||
| log.error(message); | ||
| throw new Error(message); | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖] Good catch — fixed. Moved reportStatus(jobInfo, 'finish') to before the alert-on-error throw so every terminal path reports finish first, matching the permission-failure and invalid-specifier branches (which already report finish before fail() throws). Status/progress consumers now see the job finish regardless of whether it then rejects.
Every terminal path now reports `finish` before returning or throwing, matching the permission-failure and invalid-specifier branches. Prevents a rejected system job from looking unfinished to status/progress consumers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
A
run-commandjob whose command finished with an error came back fromprerenderer.runCommand()as an in-band{status:'error'}value, whichrunCommandreturned normally. Nothing threw, so the queue marked the jobresolvedand no alert fired. A persistently failing system job was therefore completely silent — that's how the daily OpenRouter model sync stayed broken (and the realm stale) for ~3 months. The permission-failure and invalid-specifier paths were swallowed the same way.What
Add a required
alertOnErrorflag toRunCommandArgs:true— an error outcome (permission failure, invalid specifier, or a command returningstatus:'error') throws, so the queue marks the jobrejectedandSentry.captureExceptions it (existingpg-queuebehavior). Set by the system OpenRouter sync (enqueueSyncOpenRouterModels).false— preserves the current in-band error result. Set by the interactive callers (bot-runner, the run-command HTTP endpoint, webhook receiver) where a command error is a normal result to hand back to the user.Required (not optional) to satisfy
JSONTypes.Object's index signature, matching theFromScratchArgs.clearLastModifiedconvention.Tests
Extend the shared run-command task tests with two cases: an errored command and a permission failure each reject the task when
alertOnErroris set (and the existing in-band cases keepalertOnError:false).Stacking
Based on
fix/openrouter-sync-realm-identifier(#5507). Retarget tomainonce that merges.🤖 Generated with Claude Code