Background Job to Run a Command in Headless Chrome#4024
Background Job to Run a Command in Headless Chrome#4024tintinthong wants to merge 40 commits intomainfrom
Conversation
Preview deployments |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5d7b24826
ℹ️ 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".
| let resultElement = container?.querySelector( | ||
| '[data-command-result]', | ||
| ) as HTMLElement | null; |
There was a problem hiding this comment.
Return the serialized command output payload
The response extractor reads result from [data-command-result], but the new command-runner template never renders that attribute, so payload.result is always null even when the route computes model.result. This drops command output from successful run-command responses and makes the result field effectively unusable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR implements a background job system to run commands in headless Chrome via the bot-runner. It introduces a new run-command task that enables bots to execute commands in a headless browser and capture results, supporting use cases like showing cards or patching card instances through Matrix events.
Changes:
- Added
run-commandtask and job infrastructure in runtime-common for executing commands via prerenderer - Implemented command-runner route in host app to render command results in headless Chrome
- Extended bot-runner timeline handler to enqueue run-command jobs from bot-trigger events
- Removed hardcoded bot trigger command type restrictions to support custom command types
- Added submission bot to AI assistant rooms and updated setup script to register multiple bot commands
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/worker.ts | Registers the new run-command task with the worker queue |
| packages/runtime-common/tasks/run-command.ts | New task implementation for running commands with permission checks |
| packages/runtime-common/tasks/index.ts | Exports run-command task |
| packages/runtime-common/jobs/run-command.ts | Job enqueueing logic for run-command jobs |
| packages/runtime-common/index.ts | Type definitions for RunCommandArgs and RunCommandResponse, adds runCommand to Prerenderer interface |
| packages/runtime-common/code-ref.ts | New assertIsResolvedCodeRef utility function |
| packages/runtime-common/bot-trigger.ts | Removes hardcoded command type validation, adds realm field requirement |
| packages/runtime-common/bot-command.ts | Removes hardcoded command type restrictions to allow any string content_type |
| packages/runtime-common/matrix-constants.ts | Removes BOT_TRIGGER_COMMAND_TYPES constant |
| packages/realm-server/prerender/prerender-app.ts | New /run-command endpoint in prerender server |
| packages/realm-server/prerender/prerenderer.ts | Implements runCommand method with browser restart fallback |
| packages/realm-server/prerender/render-runner.ts | Implements runCommandAttempt to navigate to command-runner route and extract results |
| packages/realm-server/prerender/remote-prerenderer.ts | Adds runCommand method to remote prerenderer client |
| packages/realm-server/prerender/manager-app.ts | Proxies /run-command requests to prerender servers |
| packages/realm-server/prerender/utils.ts | New buildCommandRunnerURL utility and enhanced transitionTo with query params support |
| packages/realm-server/tests/prerender-server-test.ts | Test coverage for run-command endpoint |
| packages/realm-server/tests/prerender-proxy-test.ts | Mock prerenderer updated to support command kind |
| packages/realm-server/tests/prerender-manager-test.ts | Test coverage for command proxying in manager |
| packages/realm-server/tests/definition-lookup-test.ts | Mock prerenderer updated with runCommand stub |
| packages/host/app/router.ts | New command-runner route definition |
| packages/host/app/routes/command-runner.ts | Route implementation that executes commands and tracks state |
| packages/host/app/templates/command-runner.gts | Template that renders command results with prerender data attributes |
| packages/host/app/commands/show-card.ts | Updated to return CardDef for bot-runner compatibility |
| packages/host/app/commands/index.ts | Registers new bot-request commands and shimming |
| packages/host/app/commands/create-ai-assistant-room.ts | Invites submission bot to new AI assistant rooms |
| packages/host/app/commands/bot-requests/send-bot-trigger-event.ts | Moved to bot-requests folder, adds realm to required fields |
| packages/host/app/commands/bot-requests/create-show-card-request.ts | New command to request showing a card via bot runner |
| packages/host/app/commands/bot-requests/create-listing-pr-request.ts | Moved to bot-requests folder, adds realm parameter |
| packages/host/tests/acceptance/commands-test.gts | Test coverage for command-runner route |
| packages/host/tests/integration/commands/send-bot-trigger-event-test.gts | Updated tests to include realm field and allow custom types |
| packages/bot-runner/main.ts | Passes queuePublisher to timeline handler |
| packages/bot-runner/lib/timeline-handler.ts | Implements command enqueueing from bot-trigger events with URL-to-CodeRef conversion |
| packages/bot-runner/tests/bot-runner-test.ts | Updated test fixture with realm field |
| packages/matrix/scripts/setup-submission-bot.ts | Registers multiple bot commands (show-card, patch-card-instance, create-listing-pr) |
| packages/experiments-realm/commands/create-show-card-request.ts | Experiments realm version of create-show-card-request command |
| packages/experiments-realm/commands/create-patch-card-instance-request.ts | New command to request patching card instances |
| packages/experiments-realm/bot-request-demo.gts | Demo card with tabbed UI for testing bot requests |
| packages/experiments-realm/BotRequestDemo/bot-request-demo.json | Card instance data for demo |
| packages/base/matrix-event.gts | Removes hardcoded bot trigger command types, adds realm to BotTriggerContent |
| packages/base/command.gts | New CreateShowCardRequestInput type and realm field on SendBotTriggerEventInput |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/experiments-realm/commands/create-patch-card-instance-request.ts
Outdated
Show resolved
Hide resolved
Host Test Results 1 files ± 0 1 suites ±0 1h 37m 14s ⏱️ + 4m 46s Results for commit bd8d257. ± Comparison against base commit 18e013d. This pull request removes 1 and adds 14 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
| // TODO: boxel-host commands are not exposed internally as HTTP URLs; they | ||
| // are only available via module specifiers, so we map those URLs to code refs. |
There was a problem hiding this comment.
Perhaps, this can be removed if we treat the url as an import specifier
packages/host/app/commands/bot-requests/create-show-card-request.ts
Outdated
Show resolved
Hide resolved
| @field roomId = contains(StringField); | ||
| @field type = contains(StringField); | ||
| @field input = contains(JsonField); | ||
| @field realm = contains(StringField); |
There was a problem hiding this comment.
It seems that the worker task requires a realm to be known to check for permissions. So we just constantly send a realm from UI
| let response: RunCommandResponse = { | ||
| status: payload.status, | ||
| result: payload.result ?? undefined, | ||
| error: payload.error ?? undefined, | ||
| }; |
There was a problem hiding this comment.
I realise payload.result is the serialized json. There is also .value which is the actual card instance. Currently, there is no meaning to this as we are not using the rendering of the command anywhere other than the command-runner route.
So I'm not sure what is best to return here
There was a problem hiding this comment.
it's up to you what you want to return here. it looks like a command can return either a card or a string. you should have optional properties for these 2 different types of results.
| let queryParams = transition?.to?.queryParams ?? {}; | ||
| let command = parseResolvedCodeRef(getQueryParam(queryParams, 'command')); | ||
| let commandInput = parseJson(getQueryParam(queryParams, 'input')); |
There was a problem hiding this comment.
this is not a good pattern: you should add the command and the input as route segments. let's not use query params for our routes. so your route should look like:
this.route('command-runner', { path: '/command-runner/:command/:input/:nonce' });
|
MY BAD! I just noticed acceptance-test/commands/test-.gts. this is exactly what I wanted to see 👍 |
| <CardContainer class='command-runner-result'> | ||
| <CardRenderer @card={{@model.value}} @format='isolated' /> | ||
| </CardContainer> |
There was a problem hiding this comment.
does a command always return a card?
There was a problem hiding this comment.
not always. but this template renders conditionally
| assert.notOk(res.body.data.attributes.error, 'no command error'); | ||
| assert.ok(res.body.meta?.timing?.totalMs >= 0, 'has timing'); | ||
| assert.ok(res.body.meta?.pool?.pageId, 'has pool.pageId'); | ||
| }); |
There was a problem hiding this comment.
a command can return a card or it can return a string result. you need to make sure that you test the ability for the prerenderer to capture these 2 different types of responses from a command
There was a problem hiding this comment.
prerender only ever returns the resultCardString -- which is the serialized json
| } | ||
| let url = `${origin}/command-runner/${encodeURIComponent(nonce)}?command=${encodedCommand}`; | ||
| if (encodedInput) { | ||
| url += `&input=${encodedInput}`; |
There was a problem hiding this comment.
Yep updated to refactor
f3b3dd0 to
9613119
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 52 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import type * as JSONTypes from 'json-typescript'; | ||
|
|
||
| import type { Task } from './index'; | ||
|
|
||
| import { | ||
| fetchRealmPermissions, | ||
| jobIdentity, | ||
| type RunCommandResponse, | ||
| ensureFullMatrixUserId, | ||
| ensureTrailingSlash, | ||
| } from '../index'; | ||
|
|
||
| export interface RunCommandArgs extends JSONTypes.Object { | ||
| realmURL: string; | ||
| realmUsername: string; | ||
| runAs: string; | ||
| command: string; | ||
| commandInput: JSONTypes.Object | null; | ||
| } | ||
|
|
||
| export { runCommand }; | ||
|
|
||
| const runCommand: Task<RunCommandArgs, RunCommandResponse> = ({ | ||
| reportStatus, | ||
| log, | ||
| dbAdapter, | ||
| prerenderer, | ||
| createPrerenderAuth, | ||
| matrixURL, | ||
| }) => | ||
| async function (args) { | ||
| let { jobInfo, realmURL, realmUsername, runAs, command, commandInput } = | ||
| args; | ||
| log.debug( | ||
| `${jobIdentity(jobInfo)} starting run-command for job: ${JSON.stringify({ | ||
| realmURL, | ||
| realmUsername, | ||
| runAs, | ||
| command, | ||
| })}`, | ||
| ); | ||
| reportStatus(jobInfo, 'start'); | ||
|
|
||
| let normalizedRealmURL = ensureTrailingSlash(realmURL); | ||
| let realmPermissions = await fetchRealmPermissions( | ||
| dbAdapter, | ||
| new URL(normalizedRealmURL), | ||
| ); | ||
| let runAsUserId = ensureFullMatrixUserId(runAs, matrixURL); | ||
| let userPermissions = realmPermissions[runAsUserId]; | ||
| if (!userPermissions || userPermissions.length === 0) { | ||
| let message = `${jobIdentity(jobInfo)} ${runAs} does not have permissions in ${normalizedRealmURL}`; | ||
| log.error(message); | ||
| reportStatus(jobInfo, 'finish'); | ||
| return { | ||
| status: 'error', | ||
| error: message, | ||
| }; | ||
| } | ||
|
|
||
| let auth = createPrerenderAuth(runAsUserId, { | ||
| [normalizedRealmURL]: userPermissions, | ||
| }); | ||
|
|
||
| let normalizedCommand = normalizeCommandSpecifier( | ||
| command, | ||
| normalizedRealmURL, | ||
| ); | ||
| if (!normalizedCommand) { | ||
| let message = `${jobIdentity(jobInfo)} invalid command specifier`; | ||
| log.error(message, { command, realmURL: normalizedRealmURL }); | ||
| reportStatus(jobInfo, 'finish'); | ||
| return { | ||
| status: 'error', | ||
| error: message, | ||
| }; | ||
| } | ||
|
|
||
| let result = await prerenderer.runCommand({ | ||
| realm: normalizedRealmURL, | ||
| auth, | ||
| command: normalizedCommand, | ||
| commandInput: commandInput ?? undefined, | ||
| }); | ||
|
|
||
| reportStatus(jobInfo, 'finish'); | ||
| return result; | ||
| }; | ||
|
|
||
| function normalizeCommandSpecifier( | ||
| command: string, | ||
| realmURL: string, | ||
| ): string | undefined { | ||
| let specifier = command.trim(); | ||
| if (!specifier) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Legacy bot command URLs can point at /commands/<name>/<export> on the | ||
| // realm server host. Resolve those to the target realm before prerendering. | ||
| let path = toPathname(specifier); | ||
| if (!path || !path.startsWith('/commands/')) { | ||
| return specifier; | ||
| } | ||
|
|
||
| let [commandName, exportName = 'default'] = path | ||
| .slice('/commands/'.length) | ||
| .split('/'); | ||
| if (!commandName) { | ||
| return undefined; | ||
| } | ||
| return `${ensureTrailingSlash(realmURL)}commands/${commandName}/${exportName || 'default'}`; | ||
| } | ||
|
|
||
| function toPathname(commandSpecifier: string): string | undefined { | ||
| try { | ||
| return new URL(commandSpecifier).pathname; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
There is no test coverage for the new run-command task implementation in packages/runtime-common/tasks/run-command.ts. The task handles critical security operations like permission checking and command execution, yet has no dedicated unit tests. Consider adding tests to verify permission validation, command normalization, error handling, and the integration with the prerenderer.
| @@ -18,6 +18,9 @@ Router.map(function () { | |||
| this.route('module', { path: '/module/:id/:nonce/:options' }); | |||
| this.route('connect', { path: '/connect/:origin' }); | |||
| this.route('standby'); | |||
There was a problem hiding this comment.
The command and input parameters are passed as URL path segments, which have size limitations (typically ~2000 characters in most browsers). Large command inputs could exceed this limit and cause the request to fail. Consider using POST body parameters instead of path parameters for the command input, or at least document the size limitations in the route comments.
| this.route('standby'); | |
| this.route('standby'); | |
| // NOTE: The command and input values are passed as URL path segments and are | |
| // therefore subject to browser/proxy/server URL length limits (typically | |
| // around 2000 characters in many environments). Avoid placing large command | |
| // inputs here; for large payloads, prefer using POST body parameters or | |
| // another mechanism that is not constrained by URL length. |
There was a problem hiding this comment.
this is an interesting note. i don't think that will work tho since fundamentally our only interface for the prerender server is the URL since we are relying on a chrome browser to process our input. it might mean tho that we need to de-reference command input--like if its super massive, perhaps the input can be a card instance whose id you provide as a parameter instead of the actual payload.
There was a problem hiding this comment.
codex says this:
In Chrome (Chromium), the practical max URL length is about 2 MB (roughly 2,097,152 ASCII characters).
Two important caveats:
- The address bar (omnibox) only displays about 32 KB of URL text on most platforms.
- Real-world limits are often lower because servers/proxies may reject long URLs.
6317949 to
2bfa7ec
Compare
| localStorage.setItem( | ||
| key, | ||
| JSON.stringify({ | ||
| command: commandToRun, | ||
| input, | ||
| nonce: requestNonce, | ||
| createdAt, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
as discussed we now carry set input (as well as command)inside localStorage
| let key = `${commandRequestStorageKeyPrefix}${requestId}`; | ||
| let raw = window.localStorage.getItem(key); | ||
| if (!raw) { | ||
| return undefined; | ||
| } | ||
| window.localStorage.removeItem(key); |
There was a problem hiding this comment.
as discussed, the headless chrome then picks up the localStorage key via requestId from the url
|
Here is documentation of the updated behaviour Host
|
Demo
https://www.loom.com/share/2fe9a3e58a7c459ba574b2c0f747a667
Testing Locally
bot-runnerwithpnpm start:development.pnpm start:all.packages/matrix, runpnpm setup-submission-bot.Flow Diagram
User issuing task
Example source is the experiments demo card, but the same flow is used for any
app.boxel.bot-triggerevent.flowchart TD B["Trigger 'Send Show Card Bot Request' from UI"] B --> D["CreateShowCardRequestCommand.execute()"] D --> E["SendBotTriggerEventCommand.execute()"] E --> F["sendEvent('app.boxel.bot-trigger')"] F --> G["bot-runner receives timeline event"] G --> H["enqueueRunCommandJob() to jobs table"]Command runner architecture
flowchart TD A["bot-runner"] A -->|enqueue run-command job with command string| B["jobs table"] B --> C["runtime-common worker"] C -->|runCommand task| D["remote prerenderer"] D -->|POST /run-command| E["prerender manager"] E -->|proxy| F["prerender server app"] F -->|transitionTo command-runner route| G["headless Chrome tab"] G --> H["host command-runner route"] H -->|command executes + DOM status| G G -->|capture data-prerender + data-command-result| F F --> D D --> C C --> BMatrix Event Payload (
app.boxel.bot-trigger)Bot Trigger Data Structures
Submission Bot Commands (DB)
setup-submission-botnow writes canonical scoped command specifiers intobot_commands.command.[ { "name": "create-listing-pr", "command": "@cardstack/boxel-host/commands/create-listing-pr/default", "filter": { "type": "matrix-event", "event_type": "app.boxel.bot-trigger", "content_type": "create-listing-pr" } }, { "name": "show-card", "command": "@cardstack/boxel-host/commands/show-card/default", "filter": { "type": "matrix-event", "event_type": "app.boxel.bot-trigger", "content_type": "show-card" } }, { "name": "patch-card-instance", "command": "@cardstack/boxel-host/commands/patch-card-instance/default", "filter": { "type": "matrix-event", "event_type": "app.boxel.bot-trigger", "content_type": "patch-card-instance" } } ]Run Command Job Payload
RunCommandArgs.realmURLis derived fromevent.content.realm.{ "realmURL": "http://localhost:4201/experiments/", "realmUsername": "@alice:localhost", "runAs": "@alice:localhost", "command": "@cardstack/boxel-host/commands/show-card/default", "commandInput": { "cardId": "http://localhost:4201/experiments/Author/jane-doe", "format": "isolated" } }Command Normalization in
run-commandtaskstringacross bot-runner, job queue, and prerender request.runtime-common/tasks/run-command.tsnormalizes legacy realm-server URL forms (/commands/<name>/<export>) into a realm-local module specifier path when needed.ResolvedCodeRefconversion happens in the hostcommand-runnerroute when it parses:command.Host
command-runnerRouteExample (
commandandinputas path params)