feat: support automation worktree placement#1004
Merged
Merged
Conversation
Astro-Han
added
enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
|
Warning Review limit reached
More reviews will be available in 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR implements deterministic worktree placement for automation runs by introducing normalization rules, service-layer infrastructure for worktree creation and reset, and automated session context provisioning. All automation route handlers now preserve request-scoped instance context during scheduler settlement to maintain clean state management. ChangesAutomation Worktree Placement & Service Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for worktree placements in automation definitions, including input normalization, validation rules (such as restricting worktrees to git projects and fresh contexts), and automated worktree preparation during execution. However, a critical issue was identified in the runner's worktree preparation logic: if a worktree is currently in use by an active user session, the runner will still proceed to reset the worktree, leading to silent and permanent data loss of uncommitted user changes. It is recommended to safely check and release bindings, throwing an error if a user session remains active.
Deniwn22
reviewed
May 30, 2026
Deniwn22
left a comment
Deniwn22
left a comment
There was a problem hiding this comment.
Tested locally and it works. Consider extracting the retry logic into a shared utility — we have similar code in the auth module.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/src/worktree/index.ts (1)
371-381:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject empty slugs when
exactNameis set.If
nameslugifies to an empty string,base || undefineddrops back to random name generation even withexactName: true. That breaks the deterministic-placement contract and reintroduces fallback names for invalid exact placements.Suggested fix
const makeWorktreeInfo = Effect.fn("Worktree.makeWorktreeInfo")(function* (name?: string, exactName?: boolean) { const ctx = yield* InstanceState.context if (ctx.project.vcs !== "git") { throw new NotGitError({ message: "Worktrees are only supported for git projects" }) } const root = pathSvc.join(ctx.worktree, ".worktrees", "pawwork") yield* fs.makeDirectory(root, { recursive: true }).pipe(Effect.orDie) const base = name ? slugify(name) : "" + if (exactName && name && !base) { + throw new NameGenerationFailedError({ message: "Failed to generate a unique worktree name" }) + } return yield* candidate(root, base || undefined, exactName) })🤖 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/worktree/index.ts` around lines 371 - 381, In makeWorktreeInfo, when slugifying name into base ensure you reject an empty slug if exactName is true instead of passing undefined to candidate; specifically after computing const base = name ? slugify(name) : "" check if exactName && base === "" and throw a clear error (or return a failure) so deterministic placement is preserved; update the call to candidate(root, base || undefined, exactName) to only pass base when non-empty and rely on the new guard to prevent falling back to random names.
🤖 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/worktree/index.ts`:
- Around line 464-469: The createReady generator calls makeWorktreeInfo, setup
and then boot but boot currently swallows checkout/bootstrap failures by logging
and returning, allowing createReady to report success for an unprepared
worktree; modify boot (and any code paths it calls) so failures are raised as
Effect failures (e.g., throw or Effect.fail inside the Effect.fn generator)
instead of just logging/returning, so createReady will propagate the error and
not return an Info for an unbootstrapped directory; locate symbols createReady,
boot, setup, makeWorktreeInfo and the CreateInput/exactName usage to update the
error paths accordingly.
---
Outside diff comments:
In `@packages/opencode/src/worktree/index.ts`:
- Around line 371-381: In makeWorktreeInfo, when slugifying name into base
ensure you reject an empty slug if exactName is true instead of passing
undefined to candidate; specifically after computing const base = name ?
slugify(name) : "" check if exactName && base === "" and throw a clear error (or
return a failure) so deterministic placement is preserved; update the call to
candidate(root, base || undefined, exactName) to only pass base when non-empty
and rely on the new guard to prevent falling back to random names.
🪄 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: 87f24225-c351-4a1c-8b17-e8fc801e701e
📒 Files selected for processing (12)
packages/opencode/src/automation/fixtures.tspackages/opencode/src/automation/index.tspackages/opencode/src/automation/runner.tspackages/opencode/src/server/instance/automation.tspackages/opencode/src/worktree/index.tspackages/opencode/test/project/worktree-remove.test.tspackages/opencode/test/project/worktree.test.tspackages/opencode/test/server/automation-event-fixtures.test.tspackages/opencode/test/server/automation-routes.test.tspackages/opencode/test/server/automation-runner.test.tspackages/opencode/test/tool/automate.test.tspackages/sdk/js/src/v2/event-types.test-d.ts
Astro-Han
force-pushed
the
codex/i950-automation-pr5-worktree
branch
from
ace0483 to
2250775
Compare
13 tasks
Astro-Han
added a commit
that referenced
this pull request
May 31, 2026
Root cause: - PR #1004 forked the whole worktree start-script task, leaving no handshake for the middle state where the command has spawned but has not exited. - Windows scheduling exposed callers continuing before command launch, while short test waits misclassified slow runner setup as waiting on command exit. Changes: - Add a Deferred launch barrier so worktree create/reset waits only until the configured start command reaches the spawn boundary. - Keep stdout/stderr draining and process exit tracking in the background for long-lived start commands. - Add deterministic worktree regression coverage and relax Windows-sensitive automation runner waits. Verification: - git diff --check - bun --cwd packages/opencode test test/project/worktree.test.ts --timeout 30000 - OPENCODE_DB=:memory: bun --cwd packages/opencode test test/server/automation-runner.test.ts --timeout 30000 - bun --cwd packages/opencode test test/project/worktree-remove.test.ts --timeout 30000 - OPENCODE_DB=:memory: bun --cwd packages/opencode typecheck - PR checks passed, including unit-opencode - Manual windows-advisory run passed: https://github.com/Astro-Han/pawwork/actions/runs/26711660621 Review: - XHigh read-only review found no P0/P1/P2 issues and the optional Deferred<void> cleanup was applied. Follow-up: - None required.
13 tasks
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.
This was referenced Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds issue #950 PR5 backend worktree placement for automations:
where.worktreeforfreshautomations on git projectscontinue + worktreeWhy
PR1 froze the automation contract with
where.worktreepresent but rejected until PR5. PR2-4 made runs, scheduling, and persistence real. This PR flips that backend placement contract to supported so PR6 can build the Automations panel and PR7 can build create/form/template flows against a stable backend behavior.Related Issue
Closes part of #950. This is PR5 in the agreed 7-PR breakdown.
Human Review Status
Pending
Review Focus
Please focus on the placement contract (
where.worktreeas a normalized managed slug), thefresh-only validation boundary, and the runner path that prepares a worktree and updates the automation session execution context before prompting.Risk Notes
Behavior/platform: this touches git worktree creation/reset and session execution directories. It relies on the existing Worktree service canonicalization/reset behavior and leaves broader platform coverage to CI.
Lifecycle: repeated runs with the same placement release prior automation session worktree bindings before resetting the managed worktree; non-automation sessions remain protected by the existing Worktree reset guard.
Skipped checklist items:
How To Verify
Screenshots or Recordings
Not applicable. No visible UI changes.
Checklist
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.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.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
New Features
Bug Fixes