Skip to content

feat: execute automation run now#983

Merged
Astro-Han merged 12 commits into
devfrom
codex/i950-automation-pr2
May 29, 2026
Merged

feat: execute automation run now#983
Astro-Han merged 12 commits into
devfrom
codex/i950-automation-pr2

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 29, 2026

Copy link
Copy Markdown
Owner

Summary

Adds the PR2 backend execution slice for issue #950: manual automation.runNow now queues a run and starts background execution through the session prompt runner, with run-ledger updates for running, success, blocker, and hard-stop states.

Why

PR1 froze the automation contract and in-memory store but intentionally left execution stubbed. This PR keeps the next slice narrow: it proves manual Run now execution while the user is present, adds the shared blocker seam, and leaves scheduler, persistence, frontend UI, and worktree placement for later PRs.

Related Issue

Closes part of #950. This is PR2 in the agreed 7-PR breakdown.

Human Review Status

Pending

Review Focus

Please focus on the AutomationRunContext seam through SessionPrompt, the narrow Permission.ask onPending hook, and the run ledger transitions in packages/opencode/src/automation/index.ts.

Risk Notes

No scheduler, persistence, frontend, generated SDK, or worktree behavior is included. The run store remains in-memory from PR1. Visible UI/copy, platform/packaging, and docs/release-note surfaces were not touched.

How To Verify

bun run typecheck: passed
bun test ./test/server/automation-runner.test.ts ./test/server/automation-routes.test.ts ./test/permission-cleanup.test.ts ./test/permission-agent.test.ts: 50 passed, 0 failed

Screenshots or Recordings

Not applicable; no visible UI changes.

Checklist

  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

Summary by CodeRabbit

  • New Features

    • Run endpoint queues a run and starts background execution immediately; runs now execute with prompt sessions that record session results and costs.
    • Permission requests support an on-pending callback during approval flow.
  • Improvements

    • Prevents concurrent execution for the same work area; overlapping runs are stopped with a clear stop reason.
    • Runs can block for permissions/questions and resume; enforces a step cap (default 50).
    • Deleting an automation stops any active run and returns info about the stopped run.
  • Tests

    • Expanded coverage for execution, concurrency, blockers, lifecycle, deletion, and cancellation timing.

Review Change Stack

@Astro-Han Astro-Han added enhancement New feature or request P1 High priority app Application behavior and product flows harness Model harness, prompts, tool descriptions, and session mechanics labels May 29, 2026
@github-actions github-actions Bot removed the app Application behavior and product flows label May 29, 2026
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c6a04d2b-e6cc-4046-8b5a-a812a59badc4

📥 Commits

Reviewing files that changed from the base of the PR and between 39f7cee and 67dc4f0.

📒 Files selected for processing (2)
  • packages/opencode/src/automation/index.ts
  • packages/opencode/test/server/automation-runner.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/opencode/test/server/automation-runner.test.ts
  • packages/opencode/src/automation/index.ts

📝 Walkthrough

Walkthrough

Adds automation run execution with in-memory writer-key concurrency control, a RunExecutor contract, AutomationRunContext threading into session prompts (blocking/step-cap), server wiring to start background execution, and tests validating lifecycle, blockers, concurrency, and step-cap failures.

Changes

Automation Execution Feature

Layer / File(s) Summary
Automation Context & Error Types
packages/opencode/src/automation/run-context.ts
Defines AutomationRunAttendance, AutomationRunBlocker (permission/question), AutomationRunContext with block/clear callbacks and optional stepCap, AutomationStepCapError, and exports AutomationRunContextService with attended/unattended helpers and permissionOnPending.
Permission Request Callbacks
packages/opencode/src/permission/index.ts
Adds AskOptions extending AskInput with optional onPending?: (request) => Effect<void>, updates Permission.Interface.ask to accept it, and invokes onPending before publishing permission.asked.
Session Prompt Automation Integration
packages/opencode/src/session/prompt.ts
Threads AutomationRunContext into resolveTools, wires permission onPending, conditionally aborts or blocks external results based on attendance, clears automation state post-call, enforces step caps in runLoop, adds prompt runtime abort options, and exports promptWithAutomationContext.
Run State Lifecycle & Concurrency Control
packages/opencode/src/automation/index.ts
Adds activeWriters set, RunExecutor type, replaceRun/reviseRun helpers, exported markRunStarted/markRunBlocked/clearRunBlocker, getOptional, and runNowExecuting/executeRun with writer-key exclusivity, lifecycle transitions, automationSessionID handling on continue, error-code mapping, and cleanup.
Session-Based Executor
packages/opencode/src/automation/runner.ts
Exports sessionPromptExecutor implementing RunExecutor: selects/reuses session for continue, marks run started, provides attended/unattended context with block/clear handlers and stepCap:50, invokes promptWithAutomationContext, handles aborts, and returns { sessionID, result, cost }.
Server API Wiring
packages/opencode/src/server/instance/automation.ts
Replaces Automation.runNow with Automation.runNowExecuting(..., { executor: sessionPromptExecutor }) in the POST /:automationID/run handler; updates DELETE /:automationID to stop active runs and publish stopped run when present; updates OpenAPI descriptions.
Execution Lifecycle Tests
packages/opencode/test/server/automation-runner.test.ts, packages/opencode/test/server/automation-routes.test.ts
Adds tests covering run scheduling → completion, concurrency exclusivity (stopped with previous_run_awaiting_input), blocker lifecycle and clearing, state-transition hygiene, continue-session update semantics and deletion behavior, unattended override, and step-cap failure handling; updates route OpenAPI test to expect queued run response.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API
  participant Automation as Automation.runNowExecuting
  participant Executor as sessionPromptExecutor
  participant SessionPrompt
  participant State as activeWriters
  Client->>API: POST /:automationID/run
  API->>Automation: runNowExecuting(id, { executor })
  Automation->>Automation: runNow() -> scheduled
  Automation->>State: try acquire writerKey
  alt writerKey busy
    Automation->>Automation: revise new run -> stopped(previous_run_awaiting_input)
    Automation-->>Client: return scheduled run (immediate)
  else acquired
    Automation-->>Client: return scheduled run (immediate)
    Automation->>Executor: invoke with AbortSignal
    Executor->>SessionPrompt: promptWithAutomationContext(...)
    SessionPrompt->>Executor: prompt result / abort
    Executor-->>Automation: {sessionID, result, cost}
    Automation->>Automation: publish RunUpdated -> succeeded/failed/cancelled
    Automation->>State: release writerKey (finally)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Astro-Han/pawwork#960: Extends the automation run-ledger API introduced in PR #960 by adding a RunExecutor contract, concurrency control, and run lifecycle helpers.

Poem

🐰 In burrows deep the runs now queue and wait,
One writer at a time at the automation gate.
Blockers pause the hop, step caps guard the fray,
Sessions spark, then tidy up the play.
A rabbit cheers as runs complete their date.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: execute automation run now' clearly summarizes the main change: implementing background execution for manual automation runs.
Description check ✅ Passed The PR description follows the template structure with all required sections completed: Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, Screenshots, and Checklist items properly addressed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/i950-automation-pr2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces automation run execution and context management, allowing automations to run asynchronously with support for step caps, attendance tracking, and blocker handling (such as permission requests and user questions). Feedback on the changes highlights a few key issues: first, state-specific fields like blocker or stopReason must be deleted during state transitions in reviseRun to avoid strict Zod validation failures; second, the DefinitionUpdated event should be published when updating the definition's automation session ID; and finally, the block and clear handlers in the runner should be optimized to only publish updates when the run state actually changes, preventing redundant event publications.

Comment thread packages/opencode/src/automation/index.ts
Comment thread packages/opencode/src/automation/index.ts Outdated
Comment thread packages/opencode/src/automation/runner.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/opencode/src/server/instance/automation.ts (1)

233-234: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the now-stale route description.

This handler now actually executes the run in the background, so "Execution lands in a later PR." is no longer accurate for the published OpenAPI spec.

📝 Suggested wording
-        description: "Create a scheduled automation run record. Execution lands in a later PR.",
+        description: "Queue an automation run and start background execution; returns the queued run immediately.",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/server/instance/automation.ts` around lines 233 - 234,
Update the OpenAPI route description string for the handler with operationId
"automation.runNow" to reflect that the run is executed in the background rather
than deferred; replace the stale text "Create a scheduled automation run record.
Execution lands in a later PR." with a concise, accurate message such as "Create
a scheduled automation run record and start execution in the background." so the
published spec matches the current behavior.
🧹 Nitpick comments (1)
packages/opencode/src/automation/runner.ts (1)

16-23: 💤 Low value

Prefer Effect.gen over .pipe(Effect.flatMap(...)) for these handlers.

The block/clear callbacks can be expressed with Effect.gen for clearer sequencing:

♻️ Example
-    block: (blocker) =>
-      Effect.sync(() => {
-        currentRun = Automation.markRunBlocked(currentRun, blocker)
-      }).pipe(Effect.flatMap(() => Effect.promise(() => Automation.publishRunUpdated(currentRun)))),
+    block: (blocker) =>
+      Effect.gen(function* () {
+        currentRun = Automation.markRunBlocked(currentRun, blocker)
+        yield* Effect.promise(() => Automation.publishRunUpdated(currentRun))
+      }),

As per coding guidelines: "Use Effect.gen(function* () { ... }) for Effect composition".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/automation/runner.ts` around lines 16 - 23, The block
and clear handlers use Effect.sync(...).pipe(Effect.flatMap(...)) but should be
rewritten using Effect.gen for clearer sequencing; replace the current block and
clear implementations so they use Effect.gen(function* () { ... }) to update
currentRun via Automation.markRunBlocked(currentRun, blocker) /
Automation.clearRunBlocker(currentRun) and then yield
Automation.publishRunUpdated(currentRun), ensuring you update the outer
currentRun variable inside the generator and return the published effect rather
than piping with Effect.flatMap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/opencode/src/automation/index.ts`:
- Line 583: The code uses a possibly stale variable definition (captured before
awaiting executor(...)) when calling setDefinitionAutomationSession(definition,
prepared.sessionID), risking overwriting concurrent updates; before calling
setDefinitionAutomationSession, re-fetch the latest definition (e.g., via the
same repository/service used elsewhere) and use that fresh object (so computing
revision: latest.revision + 1) and then pass the updated definition to
setDefinitionAutomationSession to avoid clobbering concurrent changes.

In `@packages/opencode/src/automation/run-context.ts`:
- Around line 35-40: The attended/unattended factory functions
(AutomationRunContext.attended and .unattended) currently spread input after the
literal attendance field, allowing any runtime attendance present on input to
override the intended mode; change both factories to spread input first and then
set attendance (i.e., return { ...input, attendance: "attended" } and {
...input, attendance: "unattended" }) so the constructor literal always wins and
runtime-attached attendance on the passed-in context cannot override the
intended mode.

In `@packages/opencode/src/automation/runner.ts`:
- Around line 14-25: The issue is that you create context with
AutomationRunContext.attended(...) then wrap it via
AutomationRunContext.unattended(context), which spreads the attended object over
the unattended literal and reverts attendance to "attended"; fix by constructing
the handlers object (with block and clear implementations that update currentRun
and call Automation.publishRunUpdated) once as, e.g., handlers = { stepCap: 50,
block: ..., clear: ... }, then call either
AutomationRunContext.attended(handlers) or
AutomationRunContext.unattended(handlers) based on the attendance variable when
assigning scoped so the attendance tag is set correctly.

---

Outside diff comments:
In `@packages/opencode/src/server/instance/automation.ts`:
- Around line 233-234: Update the OpenAPI route description string for the
handler with operationId "automation.runNow" to reflect that the run is executed
in the background rather than deferred; replace the stale text "Create a
scheduled automation run record. Execution lands in a later PR." with a concise,
accurate message such as "Create a scheduled automation run record and start
execution in the background." so the published spec matches the current
behavior.

---

Nitpick comments:
In `@packages/opencode/src/automation/runner.ts`:
- Around line 16-23: The block and clear handlers use
Effect.sync(...).pipe(Effect.flatMap(...)) but should be rewritten using
Effect.gen for clearer sequencing; replace the current block and clear
implementations so they use Effect.gen(function* () { ... }) to update
currentRun via Automation.markRunBlocked(currentRun, blocker) /
Automation.clearRunBlocker(currentRun) and then yield
Automation.publishRunUpdated(currentRun), ensuring you update the outer
currentRun variable inside the generator and return the published effect rather
than piping with Effect.flatMap.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: deead158-6dea-4c6c-924c-d4fae4b07012

📥 Commits

Reviewing files that changed from the base of the PR and between 168950d and 57a93fa.

📒 Files selected for processing (8)
  • packages/opencode/src/automation/index.ts
  • packages/opencode/src/automation/run-context.ts
  • packages/opencode/src/automation/runner.ts
  • packages/opencode/src/permission/index.ts
  • packages/opencode/src/server/instance/automation.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/test/server/automation-routes.test.ts
  • packages/opencode/test/server/automation-runner.test.ts

Comment thread packages/opencode/src/automation/index.ts Outdated
Comment thread packages/opencode/src/automation/run-context.ts
Comment thread packages/opencode/src/automation/runner.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in 1150838. In addition to the inline fixes, I also updated the stale runNow OpenAPI description so it no longer says execution is deferred to a later PR.

@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in d5d233b. The remaining P1 boundaries are now covered: active writer locking is keyed by project/execution directory instead of automation ID, continue runs only bind automationSessionID when the definition still exists, edit-during-run preserves title/prompt changes, delete-during-run does not revive the definition, and the runNow OpenAPI description now says it returns the queued run immediately while background execution starts.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/opencode/test/server/automation-runner.test.ts (1)

15-21: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix temp-dir disposal ordering in withAutomation (return await)

Instance.provide returns the async result of fn, but return Instance.provide(...) exits the await using scope immediately, allowing tmp disposal to run before that Promise settles. Use return await so the temp git dir stays alive for the full automation run.

🛡️ Proposed fix
 async function withAutomation<T>(fn: (projectID: ProjectID) => Promise<T>) {
   await using tmp = await tmpdir({ git: true })
-  return Instance.provide({
+  return await Instance.provide({
     directory: tmp.path,
     fn: () => fn(Instance.project.id),
   })
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/test/server/automation-runner.test.ts` around lines 15 -
21, The withAutomation helper returns Instance.provide(...) while inside an
"await using tmp = await tmpdir(...)" scope which causes tmp to be disposed
before the async result resolves; change the return to "return await
Instance.provide(...)" so the function awaits the provided Promise inside the
using scope and the tmp dir (tmp from tmpdir) remains alive for the full
automation run; locate the withAutomation function and update its return to
await the Instance.provide call (referencing symbols: withAutomation, tmp,
tmpdir, Instance.provide, Instance.project.id).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/opencode/test/server/automation-runner.test.ts`:
- Around line 15-21: The withAutomation helper returns Instance.provide(...)
while inside an "await using tmp = await tmpdir(...)" scope which causes tmp to
be disposed before the async result resolves; change the return to "return await
Instance.provide(...)" so the function awaits the provided Promise inside the
using scope and the tmp dir (tmp from tmpdir) remains alive for the full
automation run; locate the withAutomation function and update its return to
await the Instance.provide call (referencing symbols: withAutomation, tmp,
tmpdir, Instance.provide, Instance.project.id).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fd0c4984-5284-4224-aa6c-841ae4706b4c

📥 Commits

Reviewing files that changed from the base of the PR and between 57a93fa and d5d233b.

📒 Files selected for processing (5)
  • packages/opencode/src/automation/index.ts
  • packages/opencode/src/automation/run-context.ts
  • packages/opencode/src/automation/runner.ts
  • packages/opencode/src/server/instance/automation.ts
  • packages/opencode/test/server/automation-runner.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/opencode/src/server/instance/automation.ts
  • packages/opencode/src/automation/runner.ts
  • packages/opencode/src/automation/run-context.ts

@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in b66d426. Deleting an automation now cancels any in-flight run for that automation: the active AbortController is stored in automation state, remove aborts it, the run is recorded as stopped with stopReason: "cancelled", and the DELETE route publishes the stopped run before the tombstone. Added coverage to assert the executor sees the abort signal and a deleted active run cannot complete as succeeded.

@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in cc73650 for the remaining outside-diff review item: withAutomation now uses return await Instance.provide(...) so the await using tmp scope stays alive until the async automation test work finishes. I also rechecked the earlier outside-diff and nitpick items: the runNow OpenAPI description is already updated, and the runner block/clear handlers already use Effect.gen with no-op publish suppression.

@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in eda2ac4. The delete-active-automation path now propagates the automation abort signal into the real session prompt path by calling SessionPrompt.cancel(sessionID, { source: "automation.cancel" }) from sessionPromptExecutor and removing the listener after completion. Added a regression test with the production sessionPromptExecutor and a hanging SSE response: runNow starts the real prompt, Automation.remove aborts it, the run is returned as stopped/cancelled, and the session records MessageAbortedError instead of waiting for the model stream to finish.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/opencode/test/server/automation-runner.test.ts (1)

322-346: ⚡ Quick win

Use the config option instead of hand-writing opencode.json in init.

server.url.origin is already available before this tmpdir(...) call, so the dynamic baseURL can be passed straight through config. This also makes the path import (Line 1) removable.

♻️ Switch from manual write to config
-      await using tmp = await tmpdir({
-        git: true,
-        init: async (dir) => {
-          await Bun.write(
-            path.join(dir, "opencode.json"),
-            JSON.stringify({
-              $schema: "https://opencode.ai/config.json",
-              enabled_providers: ["alibaba"],
-              provider: {
-                alibaba: {
-                  options: {
-                    apiKey: "test-key",
-                    baseURL: `${server.url.origin}/v1`,
-                  },
-                },
-              },
-              agent: {
-                build: {
-                  model: "alibaba/qwen-plus",
-                },
-              },
-            }),
-          )
-        },
-      })
+      await using tmp = await tmpdir({
+        git: true,
+        config: {
+          enabled_providers: ["alibaba"],
+          provider: {
+            alibaba: {
+              options: {
+                apiKey: "test-key",
+                baseURL: `${server.url.origin}/v1`,
+              },
+            },
+          },
+          agent: {
+            build: {
+              model: "alibaba/qwen-plus",
+            },
+          },
+        },
+      })

As per coding guidelines: "Use the config option in tmpdir to write an opencode.json config file during test setup by passing a partial Config.Info object."

Confirm the tmpdir config option shape accepts these fields:

#!/bin/bash
# Inspect tmpdir options and how `config` is handled in the test fixture
fd -i 'fixture.ts' --exec rg -nP -C4 '\bconfig\b|opencode\.json|function tmpdir|tmpdir\s*[:=]' {}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/test/server/automation-runner.test.ts` around lines 322 -
346, Replace the manual opencode.json write inside the tmpdir init callback with
the tmpdir "config" option: move the JSON object currently written in the init
(the provider/agent settings using server.url.origin for baseURL and model) into
the tmpdir call's config parameter (pass a partial Config.Info object), remove
the init that uses Bun.write and the path import, and ensure tmpdir is called
with git: true and the same config values so server.url.origin is injected
directly as baseURL; keep tmpdir variable name and test flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/opencode/test/server/automation-runner.test.ts`:
- Around line 302-303: The assertion against removed.stoppedRun is vacuous;
instead subscribe to the run event bus and assert that a RunUpdated event with
state "succeeded" is NOT emitted after the abort. Replace the current check
using removed.stoppedRun with code that listens to the event stream (the same
bus used elsewhere in tests), start a short listener before calling
executeRun/abort, collect RunUpdated events for the target run id, await the
abort completion, then assert the collected events contain no RunUpdated with
state: "succeeded"; reference the existing symbols removed.stoppedRun,
executeRun, Automation.runs (explain why not used), and RunUpdated to locate the
relevant logic.

---

Nitpick comments:
In `@packages/opencode/test/server/automation-runner.test.ts`:
- Around line 322-346: Replace the manual opencode.json write inside the tmpdir
init callback with the tmpdir "config" option: move the JSON object currently
written in the init (the provider/agent settings using server.url.origin for
baseURL and model) into the tmpdir call's config parameter (pass a partial
Config.Info object), remove the init that uses Bun.write and the path import,
and ensure tmpdir is called with git: true and the same config values so
server.url.origin is injected directly as baseURL; keep tmpdir variable name and
test flow unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 745a1962-8be1-4c40-a79a-574de8e7a122

📥 Commits

Reviewing files that changed from the base of the PR and between d5d233b and eda2ac4.

📒 Files selected for processing (4)
  • packages/opencode/src/automation/index.ts
  • packages/opencode/src/automation/runner.ts
  • packages/opencode/src/server/instance/automation.ts
  • packages/opencode/test/server/automation-runner.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/opencode/src/automation/runner.ts

Comment thread packages/opencode/test/server/automation-runner.test.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in decdbb2. Verified the pre-busy cancel race: SessionRunState.cancel is a no-op when no runner is busy, so automation aborts now also flow into prompt setup via an AbortSignal passed to SessionPrompt.promptWithAutomationContext. The prompt path checks that signal before setup, after user-message setup, immediately before ensureRunning, and inside the runner work before setting busy, so a delete between run-start publication and runner registration cannot continue into LLM/tool side effects. Added a regression test that deletes after the run reaches running but before the prompt runner is busy and asserts the provider is never called and no assistant message is created.\n\nLocal verification:\n- bun test test/server/automation-runner.test.ts\n- bun test test/server/automation-routes.test.ts\n- bun run typecheck\n- git diff --check

@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in 39f7cee for the two P3 items. The mock-executor delete test now records RunUpdated events and asserts no succeeded event is published after cancellation instead of checking the stopped snapshot. The automation.delete OpenAPI description now documents that an active run is stopped and the stopped run is published before the tombstone; added an OpenAPI assertion for that wording.\n\nLocal verification:\n- bun test test/server/automation-runner.test.ts\n- bun test test/server/automation-routes.test.ts\n- bun run typecheck\n- git diff --check

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/opencode/test/server/automation-routes.test.ts (1)

20-29: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Await Instance.provide(...) in this helper.

Returning the promise directly lets the await using tmp scope dispose before the async test body fully settles, which can make these route tests race against temp-dir cleanup.

💡 Minimal fix
 async function withAutomationApp<T>(fn: (input: { app: Hono; projectID: ProjectID }) => Promise<T>) {
   await using tmp = await tmpdir({ git: true })
-  return Instance.provide({
+  return await Instance.provide({
     directory: tmp.path,
     fn: async () => {
       const app = new Hono().route("/automation", AutomationRoutes())
       app.onError(ErrorMiddleware)
       return fn({ app, projectID: Instance.project.id })
     },
   })
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/test/server/automation-routes.test.ts` around lines 20 -
29, The helper withAutomationApp returns the promise from Instance.provide
without awaiting it, allowing the await using tmp scope to dispose the temp dir
before the test body finishes; change the function to await
Instance.provide(...) so the tmpdir created by tmpdir({ git: true }) remains in
scope for the duration of the provided fn. Specifically, in withAutomationApp,
await the call to Instance.provide (instead of returning it directly) so that
the using tmp lifetime encloses the async work passed into fn; keep the rest of
the logic (creating app = new Hono().route("/automation", AutomationRoutes())
and app.onError(ErrorMiddleware)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/opencode/test/server/automation-runner.test.ts`:
- Around line 421-433: The subscription created with Bus.subscribe listening for
Automation.Event.RunUpdated can leak if the timeout path wins because
unsubscribe() is only called after the awaited Promise.race; wrap the await in a
try/finally (or attach a .finally) to ensure unsubscribe() is always invoked
regardless of success or timeout, e.g. call unsubscribe() in the finally block
around the Automation.runNowExecuting(...) + Promise.race call so the listener
is removed even when the timeout rejects.

---

Outside diff comments:
In `@packages/opencode/test/server/automation-routes.test.ts`:
- Around line 20-29: The helper withAutomationApp returns the promise from
Instance.provide without awaiting it, allowing the await using tmp scope to
dispose the temp dir before the test body finishes; change the function to await
Instance.provide(...) so the tmpdir created by tmpdir({ git: true }) remains in
scope for the duration of the provided fn. Specifically, in withAutomationApp,
await the call to Instance.provide (instead of returning it directly) so that
the using tmp lifetime encloses the async work passed into fn; keep the rest of
the logic (creating app = new Hono().route("/automation", AutomationRoutes())
and app.onError(ErrorMiddleware)) unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0127fbb3-c457-473b-9830-cb410b6977a6

📥 Commits

Reviewing files that changed from the base of the PR and between eda2ac4 and 39f7cee.

📒 Files selected for processing (5)
  • packages/opencode/src/automation/runner.ts
  • packages/opencode/src/server/instance/automation.ts
  • packages/opencode/src/session/prompt.ts
  • packages/opencode/test/server/automation-routes.test.ts
  • packages/opencode/test/server/automation-runner.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/opencode/src/automation/runner.ts

Comment thread packages/opencode/test/server/automation-runner.test.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in 722a6f6. I fixed the PR2-scoped P3: executeRun now only publishes the running update when it actually transitions the run from scheduled to running, so an executor that already marked/published running does not produce duplicate running events. Added a RunUpdated sequence test that expects [running, succeeded].\n\nI did not change the fire-and-forget supervision shape or introduce a new writer-busy stopReason in this PR: the first is a scheduler/service-layer supervision follow-up, and the second changes the run stopReason contract. Both are better handled in a later execution-runtime/contract slice rather than broadening PR2.\n\nLocal verification:\n- bun test test/server/automation-runner.test.ts\n- bun test test/server/automation-routes.test.ts\n- bun run typecheck\n- git diff --check

@Astro-Han

Copy link
Copy Markdown
Owner Author

Review follow-up pushed in 67dc4f0. The pre-busy delete regression test now wraps the timeout race in try/finally so its RunUpdated subscription is always cleaned up, including the timeout failure path.\n\nLocal verification:\n- bun test test/server/automation-runner.test.ts\n- bun run typecheck\n- git diff --check

@Astro-Han Astro-Han merged commit a82fba0 into dev May 29, 2026
35 of 37 checks passed
@Astro-Han Astro-Han deleted the codex/i950-automation-pr2 branch May 29, 2026 11:18
Astro-Han added a commit that referenced this pull request Jun 2, 2026
Closes the backend gaps left after PR1-5 so the PR6/PR7 frontend slice can
build on a complete, stable automation contract.

Goal / change boundary:
- model { providerID, modelID } now required on every AutomationDefinition;
  runner passes it through so runs no longer depend on the runtime model
  fallback chain (which drifts across restarts).
- variant? optional reasoning/effort, validated against the live provider
  catalog on create/update; invalid variant returns 422 instead of failing
  late in the run.
- stop.kind === "condition" rejected at create/update with structured
  { field: "stop", message: "unsupported_stop_condition" } (scheduler never
  schedules condition stops, so accepting them only leaked dead UI surface).
- Derived fields nextFireAt / nextFires / failureStreak populated at
  create/update and refreshed after every terminal run; scheduler re-publishes
  automation.definition.updated with a bumped revision for global-sync.
- cron validation consolidated into src/automation/cron.ts; scheduler and
  derived both consume it.
- automate tool schema picks up model/variant + Provider-backed create-time
  validation.
- Migration 20260601100000 drops pre-release rows so model can be NOT NULL.

Verification:
- CI green on 5e177ba (unit-opencode flake on an unrelated VCS-routes
  20s timeout cleared on rerun).
- Local: 129 automation tests (routes/runner/scheduler/event-fixtures/tool/
  cron) + 35 session processor tests pass.

Review follow-ups addressed:
- codex: scheduler self-loop guard keyed by id:revision (not id alone).
- P2: recordRunOutcome retries on ConflictError instead of silently dropping
  the run outcome.
- CodeRabbit: 422 create test uses a guaranteed-invalid model; fixed sleeps
  replaced with terminal-state polling.
- Test seam for the ConflictError path moved off the public Automation API
  into an internal __test_hooks module.

Residual risk / deferred (tracked, not blocking PR6):
- needs_user_input / loop_gate run-error codes remain reserved; producing
  them needs prompt-loop semantics changes.
- Session.automationID reverse lookup deferred (session-contract migration).
- stop=condition kept in the schema layer for SDK shape parity but rejected
  at validate time; collapses once a condition evaluator lands.

Linked: issue #950; follows PR #960 #983 #984 #998 #1004; unblocks PR6/PR7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request harness Model harness, prompts, tool descriptions, and session mechanics P1 High priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant