Background Job to Run a Command in Headless Chrome#4024
Background Job to Run a Command in Headless Chrome#4024tintinthong wants to merge 24 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".
| return model; | ||
| } | ||
|
|
||
| void this.#runCommand(model, command, commandInput); |
There was a problem hiding this comment.
Restrict command-runner execution to trusted callers
This route executes whatever resolved command is supplied in the URL query params (void this.#runCommand(...)) without validating caller trust or command allowlisting. Since /command-runner/:nonce is a normal GET route, a crafted link can make an authenticated user’s browser execute write-capable commands with that user’s stored session, which creates a CSRF-style arbitrary command execution path.
Useful? React with 👍 / 👎.
| 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 39m 15s ⏱️ - 4m 58s For more details on these errors, see this check. Results for commit fced14f. ± Comparison against base commit 82e0e25. This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
| }, | ||
| { | ||
| name: 'show-card', | ||
| commandURL: `${realmServerURL}/boxel-host/commands/show-card/default`, |
There was a problem hiding this comment.
perhaps this should be @boxel-host/commands which is the import specifier but in url form?
| // 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' });
| if (command) { | ||
| model.commandRef = JSON.stringify(command); | ||
| } | ||
| if (commandInput) { | ||
| model.commandInput = JSON.stringify(commandInput); | ||
| } | ||
|
|
||
| if (!command) { | ||
| model.status = 'error'; | ||
| model.error = new Error('Missing or invalid command'); | ||
| return model; | ||
| } |
There was a problem hiding this comment.
this all goes away if you use route segments for the command and command input, they are guaranteed to be there
|
MY BAD! I just noticed acceptance-test/commands/test-.gts. this is exactly what I wanted to see 👍 |
| test('command-runner route renders command result card', async function (assert) { | ||
| let commandRef = { | ||
| module: `${testRealmURL}command-runner-hello-command`, | ||
| name: 'default', | ||
| }; | ||
| let commandParam = encodeURIComponent(JSON.stringify(commandRef)); | ||
| await visit(`/command-runner/1?command=${commandParam}`); | ||
|
|
||
| await waitFor('[data-prerender][data-prerender-status="ready"]'); | ||
| assert | ||
| .dom('[data-test-command-runner-greeting]') | ||
| .includesText('Hello from command runner'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
add a test for when the command throws an error and ensure that the DOM is what you would expect for an error
| assert.ok(res.body.meta?.pool?.pageId, 'has pool.pageId'); | ||
| }); | ||
|
|
||
| test('it handles run-command request', async function (assert) { |
There was a problem hiding this comment.
Add a test for when there is an error generated by the command and assert that the prerenderer is able to capture the error from the DOM correctly and return it in the properly serialized format for a command error
| <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?
|
|
||
| class CommandRunState implements CommandInvocation<CardDefConstructor> { | ||
| @tracked status: CommandInvocation<CardDefConstructor>['status'] = 'pending'; | ||
| @tracked value: CardDef | null = null; |
There was a problem hiding this comment.
this.value and this.result are confusing for me. i think what you really mean is resultCard, resultString. can we rename this to make more sense for what they actually are. its kind of ambiguous now.
| 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
| assert | ||
| .dom('[data-test-command-runner-greeting]') | ||
| .includesText('Hello from command runner'); |
There was a problem hiding this comment.
a command can return either a card or it can return a string. which one of these is this? and where is your test for the other case?
| }, | ||
| }); | ||
|
|
||
| router.post('/run-command', async (ctxt: Koa.Context) => { |
There was a problem hiding this comment.
can we try to reuse the registerPrerenderRoute function here? you have a lot of dupe code that is covered by that function.
| } catch { | ||
| // best effort; fall back to raw page url | ||
| } | ||
| let url = `${origin}/command-runner/${encodeURIComponent(nonce)}?command=${encodedCommand}`; |
There was a problem hiding this comment.
like i mentioned before, let's not use query params for the command. make it a dynamic segment of the route--this is idiomatic ember...
| } | ||
| let url = `${origin}/command-runner/${encodeURIComponent(nonce)}?command=${encodedCommand}`; | ||
| if (encodedInput) { | ||
| url += `&input=${encodedInput}`; |
f3b3dd0 to
9613119
Compare
Demo
https://www.loom.com/share/2fe9a3e58a7c459ba574b2c0f747a667
Testing Locally
bot-runnerwithpnpm start:development.pnpm start:allpackages/matrix,pnpm setup-submission-botFlow Diagram
User issuing task
Have made an example card in experiments to simulate but commands can be called from anywhere
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 DB"]Command runner architecture
flowchart TD A["Bot Runner DB access"] -->|enqueue job| B["Queue jobs table"] B --> C["Worker runtime common"] C -->|run command task| D["Prerenderer remote"] D -->|run command request| E["Prerender Manager"] E -->|proxy to prerender server| F["Prerender App"] F -->|run command in tab| G["Headless Chrome tab"] G -->|execute command| H["Host app command-runner route"] H -->|result in DOM| G G -->|capture result| F F --> D D --> C C --> BMatrix Event Payload (app.boxel.bot-trigger)
Bot Trigger Data Structures (with realm)
Bot-runner filter
Commands registered for submission-bot
The bot runner checks if the matrix events comes thru via the filter blob
[ { "name": "create-listing-pr", "command": "http://localhost:4201/commands/create-listing-pr/default", "filter": { "type": "matrix-event", "event_type": "app.boxel.bot-trigger", "content_type": "create-listing-pr" } }, { "name": "show-card", "command": "http://localhost:4201/boxel-host/commands/show-card/default", "filter": { "type": "matrix-event", "event_type": "app.boxel.bot-trigger", "content_type": "show-card" } } ]Run Command Task
RunCommandArgs.realmURLis derived fromevent.content.realm.{ "realmURL": "http://localhost:4201/experiments/", "realmUsername": "@alice:localhost", "runAs": "@alice:localhost", "command": { "module": "@cardstack/boxel-host/commands/show-card", "name": "default" }, "commandInput": { "cardId": "http://localhost:4201/experiments/Author/jane-doe", "format": "isolated" } }Command Runner Route on Host
Example (URL + queryParams)