Skip to content

Separate automation run transport status from domain outcome - #1021

Open
amadad wants to merge 1 commit into
get-bb:mainfrom
amadad:automation-domain-status
Open

Separate automation run transport status from domain outcome#1021
amadad wants to merge 1 commit into
get-bb:mainfrom
amadad:automation-domain-status

Conversation

@amadad

@amadad amadad commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Problem

closeAutomationRunForSettledThread in plugins/automations/src/run.ts collapses every settled agent thread into a single automation_runs.status column: thread.idle always maps to "succeeded", thread.failed always maps to "failed". That status only tells you the transport outcome — did the thread reach a terminal state without crashing — not the domain outcome the automation's prompt was actually trying to report.

Concretely, thread.idle fires whenever the provider turn settles, regardless of whether the agent's own instructions succeeded. The event carries lastAssistantText, which can contain a terminal token the automation's prompt was told to emit (e.g. TASK_COMPLETE, or a failure sentinel like REFRESH_FAILED) — but the run-closing code discards it. Today the only way to learn a script-mode run's actual outcome is via output/skipReason; agent-mode runs have no equivalent, so downstream consumers (dashboards, other automations chaining off a run) are forced to scrape thread logs or guess from "succeeded" even when the agent's own prompt reported a domain failure. The workflows plugin already sidesteps this by having its own bb_workflow_result convention; automations had no equivalent.

Fix

Separate the two concerns instead of conflating them:

  • Transport status (automation_runs.status: running / succeeded / failed / skipped) keeps meaning exactly what it means today — did the run mechanism itself complete.
  • Domain status (new terminal_token column) captures the last non-empty line of the run's output when it is a bare [A-Z][A-Z0-9_]* token (extractTerminalToken in plugins/automations/src/terminal-token.ts), so an automation's prompt can report its own outcome without the platform inventing a taxonomy for it.

Changes:

  • terminal-token.ts (new): strict single-line token extractor, capped at 128 chars, only accepted from a successful transport close.
  • run-summary.ts (new): formatRunTransportLabel / formatRunDomainLabel — shared transport=… / domain=… formatting for CLI and UI.
  • run.ts: closeAutomationRunForSettledThread now takes the caller-determined transport status directly ("succeeded" | "failed") plus an optional terminalToken, instead of remapping "idle" | "failed" internally.
  • server.ts: the thread.idle handler extracts the terminal token from lastAssistantText and passes it through; thread.failed explicitly nulls it (a transport failure has no trustworthy domain signal).
  • script-runner.ts: mapScriptResultToRun extracts a terminal token from script stdout on successful runs (timeouts, non-zero exit, empty output, and wakeAgent:false skips all stay null).
  • data.ts: adds the terminal_token column via the plugin's standard sequential migration (bb.storage.migrate, same pattern every other migration in this file uses), threads it through the run row/select/insert/close paths, and nulls it whenever a run is reopened or closes non-successfully. closeAutomationRun now does its update+read as a single UPDATE … WHERE status = 'running' RETURNING … instead of an unconditional update followed by a separate read, so calling it twice on an already-closed run is a no-op that returns the first final state instead of silently overwriting it.
  • service.ts: the manual-run failure path explicitly sets terminalToken: null.
  • rpc-types.ts / cli.ts / detail-view.tsx: terminalToken flows through the RPC response schema; the CLI run table and the run detail view both render transport=… and domain=… labels (domain omitted when null).

What's intentionally left out of this PR

The commit this is drawn from also included a ~600-line preflight schema-reconciliation pass (reconcileTerminalTokenMigrationPreflight) that ran before bb.storage.migrate on every plugin startup, plus ~350 lines of tests exercising it. That code defends against a specific migration-history drift this deployment's automations DB had accumulated during its own dev/rebase history — it is not something a normal install of this plugin can get into via bb.storage.migrate's existing sequential apply-by-index mechanism (see apps/server/src/services/plugins/plugin-api.ts), and its schema/index/foreign-key introspection would actively throw on any database shape it doesn't recognize. It's excluded here as operationally-specific scar tissue, not a generalizable fix. The plain ALTER TABLE automation_runs ADD COLUMN terminal_token TEXT; migration entry is the only schema change upstream needs, consistent with how every other migration in this file is written.

Test coverage

  • plugins/automations/src/terminal-token.test.ts (new, trimmed from the larger test file above): token-extraction edge cases (strict format, trailing whitespace/CRLF, length cap, wrong-case rejection) and mapScriptResultToRun's success/timeout/non-zero-exit/skip branches; plus closeAutomationRun storage behavior — non-success statuses always null out the token, and closing an already-closed run is idempotent and preserves the first terminal state.
  • plugins/automations/src/server-harness.test.ts: end-to-end thread.idle → closed run assertion that terminalToken is populated from lastAssistantText, plus a CLI-table assertion (Transport/Domain columns, transport=succeeded, domain=TASK_COMPLETE).
  • plugins/automations/src/run-status-ui.test.tsx (new): renders AutomationDetailView with a token-bearing and a legacy (null-token) run, asserts accessible transport=…/domain=… labels and that the domain badge is omitted when there's no token.

Ran locally: pnpm exec turbo run test --filter=bb-plugin-automations — 8 test files, 63/63 passing. pnpm exec turbo run typecheck --filter=bb-plugin-automations — clean.

Production note

This fix (in its full form, including the deployment-specific migration-reconciliation code excluded from this PR) is running in production on the author's own bb deployment.

@SawyerHood

Copy link
Copy Markdown
Collaborator

Hey @amadad in the future can you open up an issue before opening a PR. I would love if you could follow the CONTRIBUTING.md. It makes it easier to align on the problem first before coupling it to a solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants