diff --git a/docs/worktree-setup-script-design-review.md b/docs/worktree-setup-script-design-review.md new file mode 100644 index 000000000..ce3dc9b7d --- /dev/null +++ b/docs/worktree-setup-script-design-review.md @@ -0,0 +1,122 @@ +╔══════════════════════════════════════════════════════════════════╗ +║ DESIGN REVIEW ║ +╠══════════════════════════════════════════════════════════════════╣ +║ Document: docs/worktree-setup-script-design.md ║ +║ Reviewer: Gemini CLI (First Principles Design Review) ║ +╠══════════════════════════════════════════════════════════════════╣ +║ VERDICT: 🟢 PROCEED WITH CAUTION (Design Updated) ║ +╚══════════════════════════════════════════════════════════════════╝ + +═══════════════════════════════════════════════════════════ +PHASE A: DESIGN CHALLENGE +═══════════════════════════════════════════════════════════ + +## Premise & Problem Assessment + +The problem diagnosis is highly accurate. Implicit execution of setup scripts creates a hostile UX when things fail (e.g., missing auth, uninstalled dependencies). Exposing this as an explicit, terminal-first user choice directly addresses Issue #238. + +## Alternative Approaches Considered + +### Alternative 1: Dedicated Background Log Panel + +- **Approach**: Run the setup script as a background process (using `node-pty` but not interactive) and stream output to a read-only UI panel in Orca. +- **How it works**: Uses a similar `exec` execution context as today, but surfaces logs. +- **Why it might be better**: Guarantees execution semantics (`set -e` equivalent) and prevents the user from accidentally typing into the terminal mid-setup and messing up the command. +- **Tradeoff**: Cannot handle interactive prompts (e.g., SSH keys, 2FA, package manager choices), which is the primary reason the design rejected background execution. +- **Effort**: 1.5x (Requires building a log viewer UI). +- **Risk**: Hanging setups due to hidden auth prompts. + +### Alternative 2: Generated Runner Script (Recommended) + +- **Approach**: Main process generates a temporary executable script (e.g., `.orca-setup.sh` or `.orca-setup.cmd`) containing the setup commands wrapped in strict error handling (`set -e`). It then spawns the PTY and injects `source .orca-setup.sh`. +- **How it works**: The terminal remains interactive, but the execution of multiline commands is handled safely by the script runner. +- **Why it might be better**: Fixes the catastrophic failure containment issue of pasting multiline commands into an interactive terminal (see Phase B). +- **Tradeoff**: Requires writing temporary files and platform-specific wrappers. +- **Effort**: 1.2x. +- **Risk**: Edge cases in path resolution or permissions for the temporary script. + +### Recommendation + +The proposed **Terminal-First** design is fundamentally the right approach for visibility and interactivity. However, the execution model is **💡 BETTER ALTERNATIVE EXISTS**: You must adopt Alternative 2 (Generated Runner Script) to safely execute multiline commands in an interactive shell. + +## UX & User Journey Issues + +### Interaction State Coverage + +| Flow | Loading | Empty | Error | Success | Partial | Notes | +| --------------- | ------- | ----- | ----- | ------- | ------- | ------------------------------------------------------------------------------------------------- | +| Create Worktree | ❌ | ✅ | ✅ | ✅ | ✅ | What does the UI show between clicking "Create" and the terminal opening? Git cloning takes time. | + +### UX Findings + +| Issue | Severity | User Impact | Suggested Fix | +| ----------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------ | +| Context Switching | P2 | If user creates Worktree A, but clicks Worktree B while Git clone is running, the terminal might steal focus or open in the wrong context. | Define focus-stealing rules: Does the newly created worktree auto-focus when creation completes? | +| Terminal Identification | P3 | User has multiple terminals open. They don't know which one is running the setup script. | Give the setup PTY a specific title or header (e.g., `[Orca Setup]`). | + +## Architectural Fit + +```text +[UI Dialog] -> IPC: worktrees:create(repoId, ..., setupDecision) + | + [Main Process] -> Git Clone + | + Returns: { worktree, shouldRunSetup, envVars } + | +[Renderer updates UI] <------+ + | +[Renderer opens PTY] <------+ (Needs to pass envVars to PTY but cannot!) + | + [PTY runs setup script] +``` + +### Data Flow (4 paths) + +```text +Happy path: [User clicks create] → [Main creates worktree] → [Renderer opens PTY] → [Setup runs] +Nil/missing: [User selects skip] → [Main creates worktree] → [Returns shouldRunSetup=false] → [No terminal] +Empty: [No setup config] → [Dialog hides setup section] → [Normal create] +Upstream error: [Git clone fails] → [Main returns error] → 💥 [UI shows error toast, setup never runs] +``` + +| Issue | Severity | Example | Why | Evidence | +| ------- | -------- | ------------------------------------------------------- | ------------------------------------------------------------------------- | --------------------------------- | +| API Gap | P0 | Renderer needs to pass `ORCA_WORKTREE_PATH` to the PTY. | The `pty:spawn` IPC handler does not accept custom environment variables. | `src/main/ipc/pty.ts` lines 65-72 | + +═══════════════════════════════════════════════════════════ +PHASE B: DESIGN AUDIT +═══════════════════════════════════════════════════════════ + +## Critical Blockers (P0/P1 - Must Fix Before Implementation) + +| Blocker | Severity | Example | Why | Evidence | +| --------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------- | +| **PTY Env Injection API** | P0 | The design states: "These existing variables should be passed to the setup command PTY". The renderer opens the PTY. | `pty:spawn` hardcodes `process.env`. The requested feature cannot be built without modifying this IPC handler to accept `env?: Record`. | `src/main/ipc/pty.ts` | +| **Multiline Failure Containment** | P0 | `orca.yaml` contains: `pnpm install \n pnpm build`. | Pasting multiline text into an interactive bash/zsh prompt executes line by line. If `pnpm install` fails, the shell will STILL execute `pnpm build`. This violates idempotency and can corrupt the environment. | Standard bash/zsh interactive behavior vs script behavior (lack of `set -e`). | +| **Hanging Script Cancellation** | P1 | Setup command hangs. User presses `Ctrl+C`. | If the command was injected as multiline text, `Ctrl+C` only cancels the _currently executing line_, not the rest of the buffered text. The script will plow forward. | Standard interactive shell buffer behavior. | +| **PTY Race Conditions** | P1 | Injecting commands via `pty.write()` on startup. | Depending on the shell (e.g., heavy `.zshrc`), writing to the PTY immediately after spawn can result in dropped characters or execution before the prompt is ready. | Known `node-pty` limitation. | + +## Unverified Assumptions + +| Assumption | Evidence Required | Severity | Example | Why | +| ---------------------- | ----------------------------------------------------------------------- | -------- | ------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------ | +| PTY Execution Strategy | A proven mechanism to execute arbitrary strings in `node-pty` robustly. | P0 | "Orca starts the setup command in that terminal" | Doing this reliably across Windows (cmd/pwsh) and Mac (bash/zsh) is notoriously difficult without wrapper scripts. | + +## Hidden Complexity + +| Hidden Issue | Why It Will Surface | Severity | Example | Evidence | +| ------------- | ------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------ | +| UI State Sync | The `setupDecision` logic | P2 | The dialog needs to resolve `getEffectiveHooks()` to know if the `Setup` section should appear, meaning the Renderer needs real-time access to the resolved repo policy. | `worktree-setup-script-design.md` -> Edge Cases -> Repo Changes While Dialog Is Open | + +═══════════════════════════════════════════════════════════ +QUESTIONS FOR THE AUTHOR +═══════════════════════════════════════════════════════════ + +1. **How exactly will the Renderer execute the string in the PTY?** + → What we need: A concrete execution strategy (e.g., wrapper script, `\r` injection) that mitigates the P0 multiline failure containment issue and race conditions. + +2. **How will `ORCA_WORKTREE_PATH` reach the PTY?** + → What we need: Explicit mention of updating `pty:spawn` in `src/main/ipc/pty.ts` to accept an `env` override payload. + +3. **What happens in the UI during the "creating..." phase?** + → What we need: A definition of the UI state between clicking the dialog and the terminal opening. If the user clicks away to another worktree, does the terminal open in the background, or does it force-switch them back? diff --git a/docs/worktree-setup-script-design.md b/docs/worktree-setup-script-design.md new file mode 100644 index 000000000..1383edd77 --- /dev/null +++ b/docs/worktree-setup-script-design.md @@ -0,0 +1,650 @@ +# Worktree Setup Command Design + +## Problem + +Issue [#238](https://github.com/stablyai/orca/issues/238) asks for two behaviors: + +1. let a repo point to a setup script +2. let the user decide whether that setup should run during worktree creation + +Orca already has a partial implementation: + +- repo-level `setup` hooks exist today +- hooks can come from `orca.yaml` or repo settings UI +- the setup hook already runs in the new worktree after creation + +What is missing is the product model: + +- the create-worktree flow does not surface setup at all +- users cannot make a per-create decision +- the current UX frames setup as a generic lifecycle hook instead of a first-class workspace setup step +- setup failures are not visible enough to be actionable + +## Research Summary + +Broader developer tooling suggests a stronger design than a simple "run hook or not" toggle. + +Conductor models setup as a repo-level command that runs in the new workspace and exposes workspace env vars. It also supports repo-committed configuration. That is the closest direct precedent. + +Remote-dev tools such as Codespaces, dev containers, and Gitpod separate environment setup from app start: + +- one-time or infrequent setup commands install dependencies and prepare the workspace +- interactive commands start services later +- expensive setup work should be explicit, repeatable, and ideally idempotent + +Engineering blog guidance around `bin/setup` and setup scripts is consistent: + +- provide one obvious command +- make it safe to rerun +- keep it readable +- use it to prepare the environment, not to hide unrelated workflow logic + +## Goals + +- Make worktree setup explicit in Orca. +- Preserve the existing hook plumbing instead of inventing a second execution system. +- Let the user decide whether setup runs for each new worktree. +- Keep repo-committed configuration possible. +- Improve compatibility with Conductor-style setup commands where it is low-cost and safe. +- Set a clearer product boundary between "prepare this worktree" and "run my app." + +## Non-Goals + +- Adding a full task runner or process manager UI +- Supporting multiple named setup phases in v1 +- Parsing multiple repo config formats with complicated precedence rules +- Replacing the existing archive hook model +- Automatically starting long-running dev servers as part of worktree creation + +## Design Principles + +- Reuse the existing hook pipeline. +- Treat setup as a repo-level setup command, not as a path-only script field. +- Put the user decision in the create-worktree flow. +- Make the default policy configurable per repo. +- Require setup commands to be safe to rerun. +- Prefer cross-platform command guidance over shell-specific examples. +- Make failure visible enough that the user can recover without guessing. + +## Terminology + +This feature should be framed in the product as a `Setup Command` or `Setup Command`, not just a generic hook. + +Why: + +- "hook" is implementation language +- "setup" better communicates that the command prepares a fresh worktree +- it aligns better with established patterns like `bin/setup`, devcontainer lifecycle commands, and Gitpod init tasks + +Internally, the existing `setup` hook plumbing can remain. + +## Current State In Orca + +Relevant code: + +- [src/main/hooks.ts](../src/main/hooks.ts) +- [src/main/ipc/worktrees.ts](../src/main/ipc/worktrees.ts) +- [src/renderer/src/components/sidebar/AddWorktreeDialog.tsx](../src/renderer/src/components/sidebar/AddWorktreeDialog.tsx) +- [src/renderer/src/components/settings/RepositoryPane.tsx](../src/renderer/src/components/settings/RepositoryPane.tsx) +- [src/renderer/src/components/settings/HookEditor.tsx](../src/renderer/src/components/settings/HookEditor.tsx) +- [src/shared/types.ts](../src/shared/types.ts) + +Behavior today: + +- `getEffectiveHooks(repo)` resolves `setup` from `orca.yaml` or UI settings +- `runHook('setup', worktreePath, repo)` executes the command in the new worktree +- `worktrees:create` always runs setup when an effective setup hook exists +- the create-worktree dialog has no setup visibility or opt-out +- setup result visibility is limited to internal logging + +Important nuance: + +The backend already supports command strings, which means the configured setup may invoke: + +- a package manager script like `pnpm worktree:setup` +- a Node entrypoint like `node scripts/setup-worktree.mjs` +- a repo-local shell script like `bash scripts/setup-worktree.sh` +- inline shell commands + +So the main missing feature is not command execution support. It is product framing, policy, and UX. + +## What The Ecosystem Suggests + +## 1. Keep Setup As A Command String + +Conductor, dev containers, and common setup patterns all center on a command to run, not a special "script path" object. + +This is the right model because it supports: + +- package scripts +- Node entrypoints +- shell commands +- wrappers that dispatch differently per platform + +Adding a `setupScriptPath` field would be less flexible and would bias the feature toward shell-only implementations. + +## 2. Separate Setup From App Start + +The strongest cross-product pattern is lifecycle separation: + +- setup prepares the environment +- start commands run interactive services later + +For Orca v1, the setup/setup command should be explicitly scoped to worktree preparation tasks such as: + +- install dependencies +- copy ignored env files +- initialize submodules +- run one-time codegen +- seed local config for that worktree + +It should not be positioned as the place to: + +- launch long-running servers +- start watch processes +- open ports +- manage ongoing background services + +That guidance should appear in the design and product copy even if Orca does not enforce it technically. + +## 3. Defaults Should Be Policy, Not Just Boolean State + +The initial design proposed a boolean repo default. The broader research suggests a better model: + +```ts +type SetupRunPolicy = 'ask' | 'run-by-default' | 'skip-by-default' +``` + +Why: + +- some repos should always prompt because setup is expensive or conditional +- some repos almost always want setup +- some repos rarely need setup, but the action should remain visible + +This matches the issue better than a hidden boolean because it preserves user intent at the moment of creation. + +## 4. Output Visibility Matters + +A toast-only model is too weak. + +Setup commands fail for predictable reasons: + +- missing env files +- package manager auth issues +- version mismatches +- broken scripts + +The user needs a place to inspect the output. V1 does not need a full job dashboard, but it should preserve enough output to debug failures. + +## Proposed Product UX + +## 1. Repository Settings + +Keep the current hook system, but surface setup as a first-class setup concept. + +Recommended changes: + +- relabel the `setup` section to `Setup Command` +- keep `archive` under lifecycle hooks +- add a repo-level `When creating a worktree` policy control with: + - `Ask every time` + - `Run by default` + - `Skip by default` +- add guidance that setup commands should be safe to rerun + +Recommended helper text: + +- `Runs in the new worktree after creation to prepare the environment. Prefer an idempotent command such as "pnpm worktree:setup" or "node scripts/setup-worktree.mjs". Avoid starting long-running dev servers here.` + +Why: + +- this makes the feature discoverable +- it nudges teams toward reliable command shapes +- it aligns with cross-platform support + +## 2. Create Worktree Dialog + +When an effective setup command exists for the selected repo, show a setup section: + +- label: `Setup` +- checkbox: `Run setup command after creation` +- supporting text: show whether the command comes from `orca.yaml` or UI settings +- preview: show a truncated first line or command summary + +Initial checkbox value and dialog behavior comes from the repo policy: + +- `ask` means the dialog must require an explicit choice: + - radio/select choice: `Run setup now` or `Skip for now` + - no implicit default submit path + - the create action stays disabled until the user chooses one +- `run-by-default` means the checkbox is checked. (If triggered via a "Quick Create" flow like a command palette, it may bypass the dialog entirely). +- `skip-by-default` means the checkbox is unchecked. + +Why: + +- the issue explicitly wants user choice +- expensive setup work should be visible at creation time +- `ask` should mean a real decision, not an unchecked box that silently turns into skip + +## 3. Setup Result Visibility (Open A Terminal And Run It) + +V1 should run setup in a normal integrated terminal for the new worktree, not as a hidden background task. Because the configured setup command might be a multiline script from `orca.yaml`, it cannot simply be pasted into an interactive terminal (doing so breaks idempotency if an early line fails, as the shell will continue executing subsequent lines). + +**Execution Strategy (Generated Runner Script):** + +1. The Main Process generates a temporary executable script file inside the worktree (`.git/orca/setup-runner.sh` for macOS/Linux, or `.git/orca/setup-runner.cmd` for Windows). +2. The Main Process writes the resolved multiline setup command into this script. For bash/zsh, prepend `set -e` so failures halt execution immediately. +3. After the worktree is created, if setup is enabled, Orca opens and focuses a terminal for that worktree. +4. Orca starts the generated script in that terminal (e.g., `bash .git/orca/setup-runner.sh` or `cmd.exe /c .git\\orca\\setup-runner.cmd`). +5. Success, failure, and interactive prompts (like SSH keys) are handled directly in the terminal safely. + +**Post-Execution UX:** + +- the terminal remains a normal, fully interactive shell +- when the command finishes, the user is back at the shell prompt in that same terminal +- the user can inspect output, press `Up Arrow` to retry, or run whatever they want next + +Non-goal: + +- a background jobs system +- durable setup-session recovery across reloads +- custom log persistence or special transcript storage + +## Proposed Data Model + +Do not add `setupScriptPath`. + +Keep the setup command as a string and add a run policy: + +```ts +type SetupRunPolicy = 'ask' | 'run-by-default' | 'skip-by-default' + +type RepoHookSettings = { + mode: 'auto' | 'override' + setupRunPolicy?: SetupRunPolicy // Defaults to 'run-by-default' if undefined + scripts: { + setup: string + archive: string + } +} +``` + +Rationale: + +- the command model already solves "point to a script" and more +- policy is the missing product state +- `setupRunPolicy` being optional ensures backward compatibility with existing configs (migrating gracefully by defaulting to `run-by-default`) + +## Proposed IPC/API Changes + +Extend the create-worktree request: + +```ts +type CreateWorktreeArgs = { + repoId: string + name: string + baseBranch?: string + setupDecision?: 'inherit' | 'run' | 'skip' +} +``` + +And update the return type to provide the generated runner script and environment payload: + +```ts +type CreateWorktreeResult = { + // ... existing fields + setup?: { + runnerScriptPath: string + envVars: Record + } +} +``` + +Update `pty:spawn` in `src/main/ipc/pty.ts` to accept custom environment variables so the setup terminal can receive its context: + +```ts +// Existing: ipcMain.handle('pty:spawn', (_event, args: { cols: number; rows: number; cwd?: string }) +// New: +ipcMain.handle('pty:spawn', (_event, args: { cols: number; rows: number; cwd?: string, env?: Record }) +``` + +Behavior: + +- `run` always runs setup when an effective setup command exists +- `skip` always skips setup +- `inherit` delegates resolution to the backend using repo policy +- if the resolved policy is `ask` and the caller sends `inherit` (or omits the field), the backend rejects the request with an explicit error such as `Setup decision required for this repository` + +Why: + +- the backend must own policy enforcement so non-dialog callers cannot accidentally bypass `ask` or `skip-by-default` +- a tri-state decision keeps compatibility for existing callers while still allowing the backend to reject ambiguous creates when the repo requires a choice + +## Proposed Execution Rules + +## 1. Setup Resolution + +Continue using `getEffectiveHooks(repo)` for command resolution. + +That means setup may still come from: + +- `orca.yaml` +- UI override +- UI fallback in auto mode + +## 2. Create Flow + +`worktrees:create` should: + +1. create the git worktree exactly as it does today +2. persist worktree metadata +3. resolve whether this create operation should run setup by combining `setupDecision` with the repo's `setupRunPolicy` +4. if setup should run, generate a temporary runner script (e.g., `.git/orca/setup-runner.sh`) containing the resolved command. +5. return the created worktree plus the path to the generated setup script and the env vars to inject into the PTY. + +Crucially, **the backend owns the policy decision and script generation, but the renderer owns opening the terminal and starting the visible terminal command**. + +Requirements for execution: + +- **Terminal-First:** Setup must run in a visible terminal, not through `runHook()` and not as a hidden background exec. +- **Safe Execution:** The Renderer must pass the generated runner script and environment variables to the new PTY, not raw multiline text. +- **Simple Ownership:** Orca should use the existing terminal flow. Open a terminal for the worktree, then start the setup command there. +- **Best-Effort:** If the renderer reloads before or during setup, Orca does not need to recover or resume that setup run. The user can rerun it manually. +- **Interactivity:** The user is NOT blocked from interacting with the workspace. They can browse code while the terminal runs the setup in plain view. +- **No Rollback:** Setup failure must not roll back worktree creation (the git operation already succeeded). + +Why: + +- environment preparation is best-effort workspace setupping, not git correctness +- a terminal-first approach avoids the complexity of background process management +- using the existing terminal ownership model is much simpler than inventing setup-session infrastructure + +### Terminal Behavior + +Required behavior: + +- after a successful create with setup enabled, Orca automatically switches focus to the new worktree and opens its terminal panel +- Orca starts the generated runner script in that terminal with the appropriate environment variables injected +- when the command exits, the terminal remains available as a normal shell +- Orca does not guarantee that an in-flight setup survives reloads or terminal closure + +Why: + +- this solves the actual user problem, "show me the setup and let me interact with it" +- it avoids building a second PTY lifecycle just for setup +- if setup is interrupted, retrying in a terminal is straightforward + +## 3. Idempotency Requirement + +The product should document a strong expectation that setup commands are idempotent. + +That means rerunning the command should be safe and should not corrupt the worktree. + +Examples of acceptable behavior: + +- reinstall or verify dependencies +- overwrite generated files deterministically +- copy missing env templates without deleting user-edited files + +Examples of risky behavior Orca should discourage in docs and copy: + +- unconditional destructive deletes +- long-running foreground servers +- one-off mutations that fail or duplicate state on rerun + +Why: + +- users may create multiple worktrees +- users may retry after failure +- policy defaults may cause setup to run frequently + +## 4. Environment Variables + +Orca already provides: + +- `ORCA_ROOT_PATH` +- `ORCA_WORKTREE_PATH` +- `CONDUCTOR_ROOT_PATH` +- `GHOSTX_ROOT_PATH` + +These existing variables should be passed to the setup command PTY to ensure scripts have the context they need. + +## 5. Execution Environment (PTY) + +Shell scripts can hang indefinitely if they accidentally prompt for user input (e.g., auth prompts, `read -p`). + +By executing the setup command in an integrated terminal instead of a hidden background process: + +- the user can see and respond to interactive prompts natively. +- the user has full control to cancel hanging scripts via standard terminal controls (`Ctrl+C`). +- familiar, colorized output is preserved. + +## Repo-Committed Config Format + +For v1, keep `orca.yaml` as the repo-committed config surface. + +Example: + +```yaml +scripts: + setup: | + pnpm install + node scripts/setup-worktree.mjs +``` + +or: + +```yaml +scripts: + setup: | + node scripts/setup-worktree.mjs +``` + +Do not add `conductor.json` parsing in this issue. + +Why: + +- Orca already has `orca.yaml` +- loading both config files introduces precedence ambiguity +- env compatibility gives most of the practical value + +## Cross-Platform Guidance + +This feature must remain compatible with macOS, Linux, and Windows. + +Recommended command examples: + +- `pnpm worktree:setup` +- `npm run worktree:setup` +- `node scripts/setup-worktree.mjs` + +Avoid recommending only: + +- `./scripts/setup-worktree.sh` + +Why: + +- shell-script-only examples are weaker on Windows +- package scripts and Node entrypoints are easier to keep portable + +## Edge Cases + +## 1. No Setup Command Configured + +- create-worktree dialog shows no setup section +- create flow behaves as it does today without setup execution + +## 2. Repo Changes While Dialog Is Open + +- the selected repo’s effective setup state controls setup section visibility +- if the repo selection changes and the new repo has no setup command, hide the section +- if the source changes between YAML and UI fallback, update the source label accordingly + +## 3. Expensive Or Conditional Setup + +- `ask` policy keeps the choice explicit +- `skip-by-default` covers repos where setup is uncommon but still available + +## 4. Setup Failure + +- worktree stays created +- failure is surfaced to the user +- the user can inspect output +- no automatic deletion or rollback + +## 5. Re-Run After Creation / Recovery + +Because setups can fail due to transient issues (e.g., missing `.env`, VPN drops), recovery is straightforward because the user is left in a normal terminal. + +- The user can simply press `Up Arrow` and `Enter` in the terminal to retry the setup command. +- We can include a "Rerun Setup" action in the Worktree context menu later as a convenient shortcut that opens a terminal for that worktree and runs the same command again. +- This leverages the idempotency requirement to give developers an easy escape hatch when setup fails. + +## 6. UI State During Creation + +- Git cloning takes time. During creation, the "Create" button in the dialog should show a loading spinner. +- Once creation is successful, if setup is enabled, Orca should automatically switch focus to the new worktree and immediately open its terminal to surface the setup run. + +## 8. Security & Unverified Repositories + +- Automatically running setup scripts is a vector for arbitrary code execution if a user clones an untrusted repository. +- Because Orca relies on standard git cloning, if the user explicitly clicks `Run setup now`, they are opting in. However, the `run-by-default` policy must be carefully considered if Orca ever adds features to auto-clone arbitrary public repos. For v1 (managing existing trusted work repositories), defaulting to `run-by-default` is acceptable, but the UI must always display the preview of the command being run. + +## Alternatives Considered + +## 1. Background Execution with Log Tailing + +Rejected. + +Reasons: + +- running shell scripts in the background is fragile (hidden SSH prompts cause hanging). +- building robust cross-platform process cancellation is difficult. +- tailing text logs in Electron requires additional IPC streaming overhead. +- "preventing interaction" while a 5-minute setup runs creates a hostile UX. A terminal-first approach solves all of these cleanly. + +## 2. Add A Dedicated `setupScriptPath` Field + +Rejected. + +Reasons: + +- current command-string model already supports script paths +- a path-only field biases the feature toward shell-specific usage +- command strings cover package scripts, Node entrypoints, wrappers, and inline commands with one model + +## 2. Use A Boolean Default + +Rejected in favor of a policy enum. + +Reasons: + +- a boolean cannot express "always prompt" +- issue #238 is fundamentally about user choice at creation time +- policy better matches real repo variation + +## 3. Always Auto-Run Setup Like Conductor + +Rejected. + +Reasons: + +- the issue explicitly asks for user choice +- setup commands can be slow, conditional, or side-effectful + +## 4. Parse `conductor.json` + +Rejected for this issue. + +Reasons: + +- increases config precedence complexity +- not required to solve the feature request +- environment compatibility provides most of the reuse value + +## Implementation Plan + +## Main Process + +Files: + +- [src/shared/types.ts](../src/shared/types.ts) +- [src/main/hooks.ts](../src/main/hooks.ts) +- [src/main/ipc/worktrees.ts](../src/main/ipc/worktrees.ts) +- [src/main/ipc/pty.ts](../src/main/ipc/pty.ts) + +Changes: + +- add `SetupRunPolicy` +- add `setupRunPolicy?: SetupRunPolicy` to `RepoHookSettings` +- extend `worktrees:create` args with `setupDecision?: 'inherit' | 'run' | 'skip'` +- resolve effective setup behavior in `worktrees:create`, including rejecting ambiguous creates when policy is `ask` +- if setup should run, generate a temporary runner script file (e.g. `.git/orca/setup-runner.sh`) containing the resolved command with `set -e` +- return the created worktree, the generated script path, and the injected environment variables in the result payload +- update `pty:spawn` to accept custom `env` overrides +- keep hidden `runHook()` execution for archive, but do not use it for visible setup execution + +## Renderer + +Files: + +- [src/preload/index.d.ts](../src/preload/index.d.ts) +- [src/preload/index.ts](../src/preload/index.ts) +- [src/renderer/src/store/slices/worktrees.ts](../src/renderer/src/store/slices/worktrees.ts) +- [src/renderer/src/components/sidebar/AddWorktreeDialog.tsx](../src/renderer/src/components/sidebar/AddWorktreeDialog.tsx) +- [src/renderer/src/components/settings/RepositoryPane.tsx](../src/renderer/src/components/settings/RepositoryPane.tsx) +- [src/renderer/src/components/settings/HookEditor.tsx](../src/renderer/src/components/settings/HookEditor.tsx) + +Changes: + +- thread `setupDecision` through preload and store +- show policy-driven setup controls in `AddWorktreeDialog` with a loading state during creation +- for `ask`, require an explicit `Run setup now` vs `Skip for now` choice before enabling create +- for `run-by-default` and `skip-by-default`, initialize the checkbox from repo policy +- expose setup policy in repository settings +- update settings copy to emphasize setup scope and idempotency +- on successful worktree creation, automatically switch focus to the new worktree +- if setup is enabled, open a terminal for the new worktree, pass the returned custom environment variables to the PTY, and start the generated runner script + +## Tests + +Add or extend tests for: + +- `worktrees:create` skips setup when `setupDecision` is `skip` +- `worktrees:create` generates a runner script and returns path when `setupDecision` is `run` +- `worktrees:create` resolves `inherit` via repo policy +- `worktrees:create` rejects ambiguous `inherit` calls when repo policy is `ask` +- create-worktree dialog shows setup controls only when effective setup exists +- dialog requires an explicit choice when repo policy is `ask` +- dialog uses repo policy for initial state when policy is `run-by-default` or `skip-by-default` +- renderer opens a terminal and starts setup when create returns with setup enabled +- output is visible in the terminal after setup failure + +## Recommendation + +Implement this as an extension of the current hook system, but tighten the product model: + +- frame setup as a worktree setup command +- keep the command as a string +- use a repo-level run policy enum instead of a boolean default +- keep explicit per-create user choice in the dialog, and enforce `ask` in the backend instead of trusting the renderer +- **execute the setup command via a generated runner script in a normal integrated terminal** so multiline commands execute safely, and prompts, cancellation, and output are visible +- explicitly pass standard Orca context variables (`ORCA_WORKTREE_PATH`, etc.) directly into the PTY environment +- keep v1 best-effort, if the terminal is closed or the renderer reloads, the user can rerun setup manually +- document setup as idempotent environment preparation, not app startup + +## Sources + +- [Conductor environment variables](https://docs.conductor.build/tips/conductor-env) +- [Conductor workspaces and branches](https://docs.conductor.build/tips/workspaces-and-branches) +- [Conductor using monorepos](https://docs.conductor.build/tips/using-monorepos) +- [GitHub Codespaces: Introduction to dev containers](https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/adding-a-dev-container-configuration/introduction-to-dev-containers) +- [GitHub Codespaces: Configuring prebuilds](https://docs.github.com/en/codespaces/prebuilding-your-codespaces/configuring-prebuilds) +- [containers.dev supporting tools and prebuild patterns](https://containers.dev/supporting.html) +- [containers.dev prebuild guide](https://containers.dev/guide/prebuild) +- [Gitpod tasks](https://ona.com/docs/classic/user/configure/workspaces/tasks) +- [thoughtbot: Use `bin/setup` to simplify development environment setup](https://thoughtbot.com/blog/bin-setup) +- [thoughtbot: Laptop setup for an awesome development environment](https://thoughtbot.com/blog/laptop-setup-for-an-awesome-development-environment) +- [Chris Blunt: Simplifying local environment setup with `bin/setup`](https://www.chrisblunt.com/rails-simplifying-local-environment-setup/) +- [Mesi Rendon: Working environment setupper](https://mesirendon.com/articles/working-environment-setuper/) +- [Nathan Onn: Git worktrees and setup friction in multi-agent workflows](https://www.nathanonn.com/how-i-vibe-code-with-3-ai-agents-using-git-worktrees-without-breaking-anything/) diff --git a/src/main/hooks-runner.test.ts b/src/main/hooks-runner.test.ts new file mode 100644 index 000000000..5b22ecb6f --- /dev/null +++ b/src/main/hooks-runner.test.ts @@ -0,0 +1,77 @@ +import type { Repo } from '../shared/types' + +import { describe, expect, it, vi } from 'vitest' + +const { execFileSyncMock } = vi.hoisted(() => ({ + execFileSyncMock: vi.fn() +})) + +vi.mock('fs', () => ({ + readFileSync: vi.fn(), + existsSync: vi.fn(), + mkdirSync: vi.fn(), + writeFileSync: vi.fn(), + chmodSync: vi.fn() +})) + +vi.mock('child_process', () => ({ + exec: vi.fn(), + execFileSync: execFileSyncMock +})) + +describe('createSetupRunnerScript', () => { + const makeRepo = () => + ({ + id: 'test-id', + path: '/test/repo', + displayName: 'Test Repo', + badgeColor: '#000', + addedAt: Date.now() + }) as unknown as Repo + + it('writes a fail-fast Windows runner that returns after batch commands', async () => { + const fs = await import('fs') + const originalPlatform = process.platform + + execFileSyncMock.mockReturnValue('C:\\repo\\.git\\worktrees\\feature\\orca\\setup-runner.cmd') + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32' + }) + + try { + const { createSetupRunnerScript } = await import('./hooks') + const result = createSetupRunnerScript( + makeRepo(), + 'C:\\repo\\feature', + 'pnpm install\npnpm build' + ) + + expect(result).toEqual({ + runnerScriptPath: 'C:\\repo\\.git\\worktrees\\feature\\orca\\setup-runner.cmd', + envVars: expect.objectContaining({ + ORCA_ROOT_PATH: '/test/repo', + ORCA_WORKTREE_PATH: 'C:\\repo\\feature' + }) + }) + expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith( + 'C:\\repo\\.git\\worktrees\\feature\\orca\\setup-runner.cmd', + [ + '@echo off', + 'setlocal EnableExtensions', + 'call pnpm install', + 'if errorlevel 1 exit /b %errorlevel%', + 'call pnpm build', + 'if errorlevel 1 exit /b %errorlevel%', + '' + ].join('\r\n'), + 'utf-8' + ) + } finally { + Object.defineProperty(process, 'platform', { + configurable: true, + value: originalPlatform + }) + } + }) +}) diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index df88cf8cc..f1b3bb884 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -1,3 +1,5 @@ +import type { Repo } from '../shared/types' + import { describe, expect, it, vi } from 'vitest' import { parseOrcaYaml } from './hooks' @@ -12,7 +14,8 @@ const { execMock } = vi.hoisted(() => ({ })) vi.mock('child_process', () => ({ - exec: execMock + exec: execMock, + execFileSync: vi.fn() })) describe('parseOrcaYaml', () => { @@ -60,6 +63,17 @@ describe('parseOrcaYaml', () => { expect(parseOrcaYaml(yaml)).toBeNull() }) + it('parses YAML with inline scalar scripts', () => { + const yaml = `scripts:\n setup: npm install\n archive: sleep 5\n` + const result = parseOrcaYaml(yaml) + expect(result).toEqual({ + scripts: { + setup: 'npm install', + archive: 'sleep 5' + } + }) + }) + it('returns null when scripts block has no setup or archive', () => { const yaml = `scripts:\n unknown: |\n echo "nope"\n` expect(parseOrcaYaml(yaml)).toBeNull() @@ -93,25 +107,27 @@ describe('parseOrcaYaml', () => { describe('getEffectiveHooks', () => { // We need to dynamically import after mocking const makeRepo = (hookSettings?: { - mode: 'auto' | 'override' - scripts: { setup: string; archive: string } - }) => ({ - id: 'test-id', - path: '/test/repo', - displayName: 'Test Repo', - badgeColor: '#000', - addedAt: Date.now(), - hookSettings - }) + mode?: 'auto' | 'override' + setupRunPolicy?: 'ask' | 'run-by-default' | 'skip-by-default' + scripts?: { setup: string; archive: string } + }) => + ({ + id: 'test-id', + path: '/test/repo', + displayName: 'Test Repo', + badgeColor: '#000', + addedAt: Date.now(), + hookSettings + }) as unknown as Repo - it('auto mode with yaml hooks uses yaml hooks', async () => { + it('uses hooks from orca.yaml when present', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(true) vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo "yaml setup"\n') // Re-import to pick up mocks const { getEffectiveHooks } = await import('./hooks') - const repo = makeRepo({ mode: 'auto', scripts: { setup: '', archive: '' } }) + const repo = makeRepo() const result = getEffectiveHooks(repo) expect(result).toEqual({ @@ -121,25 +137,26 @@ describe('getEffectiveHooks', () => { }) }) - it('auto mode with no yaml hooks but UI scripts uses UI scripts', async () => { + it('falls back to legacy UI hooks when yaml is missing', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(false) const { getEffectiveHooks } = await import('./hooks') const repo = makeRepo({ - mode: 'auto', - scripts: { setup: 'echo "ui setup"', archive: '' } + mode: 'override', + scripts: { setup: 'echo "legacy ui setup"', archive: 'echo "legacy archive"' } }) const result = getEffectiveHooks(repo) expect(result).toEqual({ scripts: { - setup: 'echo "ui setup"' + setup: 'echo "legacy ui setup"', + archive: 'echo "legacy archive"' } }) }) - it('override mode always uses UI scripts, ignores yaml', async () => { + it('ignores legacy UI override settings when yaml exists', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(true) vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo "yaml setup"\n') @@ -153,7 +170,27 @@ describe('getEffectiveHooks', () => { expect(result).toEqual({ scripts: { - setup: 'echo "ui override"' + setup: 'echo "yaml setup"' + } + }) + }) + + it('falls back per hook when orca.yaml defines only one command', async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n archive: |\n echo "yaml archive"\n') + + const { getEffectiveHooks } = await import('./hooks') + const repo = makeRepo({ + mode: 'override', + scripts: { setup: 'echo "legacy setup"', archive: 'echo "legacy archive"' } + }) + const result = getEffectiveHooks(repo) + + expect(result).toEqual({ + scripts: { + setup: 'echo "legacy setup"', + archive: 'echo "yaml archive"' } }) }) @@ -172,16 +209,18 @@ describe('getEffectiveHooks', () => { describe('runHook', () => { const makeRepo = (hookSettings?: { - mode: 'auto' | 'override' - scripts: { setup: string; archive: string } - }) => ({ - id: 'test-id', - path: '/test/repo', - displayName: 'Test Repo', - badgeColor: '#000', - addedAt: Date.now(), - hookSettings - }) + mode?: 'auto' | 'override' + setupRunPolicy?: 'ask' | 'run-by-default' | 'skip-by-default' + scripts?: { setup: string; archive: string } + }) => + ({ + id: 'test-id', + path: '/test/repo', + displayName: 'Test Repo', + badgeColor: '#000', + addedAt: Date.now(), + hookSettings + }) as unknown as Repo it('uses the Windows command shell when running hooks', async () => { execMock.mockImplementation((_script, _options, callback) => { @@ -189,6 +228,10 @@ describe('runHook', () => { return {} as never }) + const fs = await import('fs') + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo hello\n') + const originalPlatform = process.platform const originalComSpec = process.env.ComSpec @@ -200,14 +243,7 @@ describe('runHook', () => { try { const { runHook } = await import('./hooks') - const result = await runHook( - 'setup', - 'C:\\repo\\worktree', - makeRepo({ - mode: 'override', - scripts: { setup: 'echo hello', archive: '' } - }) - ) + const result = await runHook('setup', 'C:\\repo\\worktree', makeRepo()) expect(result).toEqual({ success: true, output: '' }) expect(execMock).toHaveBeenCalledWith( @@ -237,6 +273,10 @@ describe('runHook', () => { return {} as never }) + const fs = await import('fs') + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo hello\n') + const originalPlatform = process.platform const originalShell = process.env.SHELL @@ -248,14 +288,7 @@ describe('runHook', () => { try { const { runHook } = await import('./hooks') - const result = await runHook( - 'setup', - '/repo/worktree', - makeRepo({ - mode: 'override', - scripts: { setup: 'echo hello', archive: '' } - }) - ) + const result = await runHook('setup', '/repo/worktree', makeRepo()) expect(result).toEqual({ success: true, output: '' }) expect(execMock).toHaveBeenCalledWith( @@ -279,3 +312,41 @@ describe('runHook', () => { } }) }) + +describe('shouldRunSetupForCreate', () => { + const makeRepo = (setupRunPolicy?: 'ask' | 'run-by-default' | 'skip-by-default') => + ({ + id: 'test-id', + path: '/test/repo', + displayName: 'Test Repo', + badgeColor: '#000', + addedAt: Date.now(), + hookSettings: { + mode: 'auto', + setupRunPolicy, + scripts: { setup: '', archive: '' } + } + }) as unknown as Repo + + it('requires an explicit decision when the repo policy is ask', async () => { + const { shouldRunSetupForCreate } = await import('./hooks') + + expect(() => shouldRunSetupForCreate(makeRepo('ask'))).toThrow( + 'Setup decision required for this repository' + ) + }) + + it('uses the repo default when the caller inherits', async () => { + const { shouldRunSetupForCreate } = await import('./hooks') + + expect(shouldRunSetupForCreate(makeRepo('run-by-default'))).toBe(true) + expect(shouldRunSetupForCreate(makeRepo('skip-by-default'))).toBe(false) + }) + + it('lets the caller override the repo default per create', async () => { + const { shouldRunSetupForCreate } = await import('./hooks') + + expect(shouldRunSetupForCreate(makeRepo('skip-by-default'), 'run')).toBe(true) + expect(shouldRunSetupForCreate(makeRepo('run-by-default'), 'skip')).toBe(false) + }) +}) diff --git a/src/main/hooks.ts b/src/main/hooks.ts index c1cfdad19..e9204d958 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -1,11 +1,16 @@ -import { readFileSync, existsSync } from 'fs' -import { join } from 'path' -import { exec } from 'child_process' +import { readFileSync, existsSync, mkdirSync, writeFileSync, chmodSync } from 'fs' +import { dirname, join } from 'path' +import { exec, execFileSync } from 'child_process' import { getDefaultRepoHookSettings } from '../shared/constants' -import type { OrcaHooks, Repo } from '../shared/types' +import type { + OrcaHooks, + Repo, + SetupDecision, + SetupRunPolicy, + WorktreeSetupLaunch +} from '../shared/types' const HOOK_TIMEOUT = 120_000 // 2 minutes -type HookName = keyof OrcaHooks['scripts'] function getHookShell(): string | undefined { if (process.platform === 'win32') { @@ -42,15 +47,15 @@ export function parseOrcaYaml(content: string): OrcaHooks | null { break } - // Indented key like " setup: |" or " archive: |" - const keyMatch = line.match(/^ (setup|archive):\s*\|?\s*$/) + // Indented key like " setup: |" or " archive: |" or " setup: echo hello" + const keyMatch = line.match(/^ (setup|archive):\s*(\|)?\s*(.*)$/) if (keyMatch) { // Save previous key if (currentKey) { hooks.scripts[currentKey] = currentValue.trimEnd() } currentKey = keyMatch[1] as 'setup' | 'archive' - currentValue = '' + currentValue = keyMatch[3] ? `${keyMatch[3]}\n` : '' continue } @@ -96,35 +101,127 @@ export function hasHooksFile(repoPath: string): boolean { } export function getEffectiveHooks(repo: Repo): OrcaHooks | null { - const defaults = getDefaultRepoHookSettings() const yamlHooks = loadHooks(repo.path) - const repoSettings = { - ...defaults, - ...repo.hookSettings, - scripts: { - ...defaults.scripts, - ...repo.hookSettings?.scripts - } - } + const legacySetup = repo.hookSettings?.scripts.setup?.trim() + const legacyArchive = repo.hookSettings?.scripts.archive?.trim() + const setup = yamlHooks?.scripts.setup?.trim() || legacySetup + const archive = yamlHooks?.scripts.archive?.trim() || legacyArchive - const hooks: OrcaHooks = { scripts: {} } - - for (const hookName of ['setup', 'archive'] as HookName[]) { - const yamlScript = yamlHooks?.scripts[hookName]?.trim() - const uiScript = repoSettings.scripts[hookName].trim() - - const autoScript = yamlScript || uiScript || undefined - const effectiveScript = repoSettings.mode === 'auto' ? autoScript : uiScript || undefined - - if (effectiveScript) { - hooks.scripts[hookName] = effectiveScript - } - } - - if (!hooks.scripts.setup && !hooks.scripts.archive) { + if (!setup && !archive) { return null } - return hooks + + // Why: `orca.yaml` is the preferred source going forward, but existing users may + // still have setup/archive commands persisted only in repo settings. Resolve each + // hook independently so a repo that has only migrated one command into `orca.yaml` + // does not silently lose the other legacy hook until the migration is complete. + return { + scripts: { + ...(setup ? { setup } : {}), + ...(archive ? { archive } : {}) + } + } +} + +export function getEffectiveSetupRunPolicy(repo: Repo): SetupRunPolicy { + return repo.hookSettings?.setupRunPolicy ?? getDefaultRepoHookSettings().setupRunPolicy! +} + +export function shouldRunSetupForCreate(repo: Repo, decision: SetupDecision = 'inherit'): boolean { + if (decision === 'run') { + return true + } + if (decision === 'skip') { + return false + } + + const policy = getEffectiveSetupRunPolicy(repo) + if (policy === 'ask') { + throw new Error('Setup decision required for this repository') + } + + return policy === 'run-by-default' +} + +export function getSetupCommandSource(repo: Repo): { source: 'yaml'; command: string } | null { + const yamlSetup = loadHooks(repo.path)?.scripts.setup?.trim() + + if (yamlSetup) { + return { source: 'yaml', command: yamlSetup } + } + + return null +} + +function getSetupEnvVars(repo: Repo, worktreePath: string): Record { + return { + ORCA_ROOT_PATH: repo.path, + ORCA_WORKTREE_PATH: worktreePath, + // Compat with conductor.json users + CONDUCTOR_ROOT_PATH: repo.path, + GHOSTX_ROOT_PATH: repo.path + } +} + +function getGitPath(cwd: string, relativePath: string): string { + return execFileSync('git', ['rev-parse', '--git-path', relativePath], { + cwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'] + }).trim() +} + +function buildWindowsRunnerScript(script: string): string { + const lines = script.replace(/\r?\n/g, '\n').split('\n') + const runnerLines = ['@echo off', 'setlocal EnableExtensions'] + + for (const rawLine of lines) { + const command = rawLine.trim() + if (!command) { + runnerLines.push('') + continue + } + + // Why: setup commands often invoke `npm`/`pnpm`, which are batch files on + // Windows. Calling one batch file from another without `call` never returns + // to later lines, and plain newline-separated commands also keep running + // after failures. Wrap each line in `call` and bail on non-zero exit codes + // so the generated runner matches the fail-fast behavior of `set -e`. + runnerLines.push(`call ${command}`) + runnerLines.push('if errorlevel 1 exit /b %errorlevel%') + } + + return `${runnerLines.join('\r\n')}\r\n` +} + +export function createSetupRunnerScript( + repo: Repo, + worktreePath: string, + script: string +): WorktreeSetupLaunch { + const envVars = getSetupEnvVars(repo, worktreePath) + const isWindows = process.platform === 'win32' + const normalizedScript = isWindows + ? script.replace(/\r?\n/g, '\r\n') + : script.replace(/\r\n/g, '\n') + // Why: linked git worktrees use a `.git` file that points at the real gitdir, + // so writing under `${worktreePath}/.git/...` fails. `git rev-parse --git-path` + // resolves the actual per-worktree git storage path safely across platforms. + const runnerScriptPath = getGitPath( + worktreePath, + isWindows ? 'orca/setup-runner.cmd' : 'orca/setup-runner.sh' + ) + + mkdirSync(dirname(runnerScriptPath), { recursive: true }) + + if (isWindows) { + writeFileSync(runnerScriptPath, buildWindowsRunnerScript(normalizedScript), 'utf-8') + } else { + writeFileSync(runnerScriptPath, `#!/usr/bin/env bash\nset -e\n${normalizedScript}\n`, 'utf-8') + chmodSync(runnerScriptPath, 0o755) + } + + return { runnerScriptPath, envVars } } /** @@ -151,11 +248,7 @@ export function runHook( shell: getHookShell(), env: { ...process.env, - ORCA_ROOT_PATH: repo.path, - ORCA_WORKTREE_PATH: cwd, - // Compat with conductor.json users - CONDUCTOR_ROOT_PATH: repo.path, - GHOSTX_ROOT_PATH: repo.path + ...getSetupEnvVars(repo, cwd) } }, (error, stdout, stderr) => { diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 84b6abfba..5f01263f4 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -67,60 +67,64 @@ export function registerPtyHandlers(mainWindow: BrowserWindow, runtime?: OrcaRun } }) - ipcMain.handle('pty:spawn', (_event, args: { cols: number; rows: number; cwd?: string }) => { - const id = String(++ptyCounter) + ipcMain.handle( + 'pty:spawn', + (_event, args: { cols: number; rows: number; cwd?: string; env?: Record }) => { + const id = String(++ptyCounter) - let shellPath: string - let shellArgs: string[] - if (process.platform === 'win32') { - shellPath = process.env.COMSPEC || 'powershell.exe' - shellArgs = [] - } else { - shellPath = process.env.SHELL || '/bin/zsh' - shellArgs = ['-l'] + let shellPath: string + let shellArgs: string[] + if (process.platform === 'win32') { + shellPath = process.env.COMSPEC || 'powershell.exe' + shellArgs = [] + } else { + shellPath = process.env.SHELL || '/bin/zsh' + shellArgs = ['-l'] + } + + const defaultCwd = + process.platform === 'win32' + ? process.env.USERPROFILE || process.env.HOMEPATH || 'C:\\' + : process.env.HOME || '/' + + const ptyProcess = pty.spawn(shellPath, shellArgs, { + name: 'xterm-256color', + cols: args.cols, + rows: args.rows, + cwd: args.cwd || defaultCwd, + env: { + ...process.env, + ...args.env, + TERM: 'xterm-256color', + COLORTERM: 'truecolor', + TERM_PROGRAM: 'Orca', + FORCE_HYPERLINK: '1' + } as Record + }) + + ptyProcesses.set(id, ptyProcess) + ptyLoadGeneration.set(id, loadGeneration) + runtime?.onPtySpawned(id) + + ptyProcess.onData((data) => { + runtime?.onPtyData(id, data, Date.now()) + if (!mainWindow.isDestroyed()) { + mainWindow.webContents.send('pty:data', { id, data }) + } + }) + + ptyProcess.onExit(({ exitCode }) => { + ptyProcesses.delete(id) + ptyLoadGeneration.delete(id) + runtime?.onPtyExit(id, exitCode) + if (!mainWindow.isDestroyed()) { + mainWindow.webContents.send('pty:exit', { id, code: exitCode }) + } + }) + + return { id } } - - const defaultCwd = - process.platform === 'win32' - ? process.env.USERPROFILE || process.env.HOMEPATH || 'C:\\' - : process.env.HOME || '/' - - const ptyProcess = pty.spawn(shellPath, shellArgs, { - name: 'xterm-256color', - cols: args.cols, - rows: args.rows, - cwd: args.cwd || defaultCwd, - env: { - ...process.env, - TERM: 'xterm-256color', - COLORTERM: 'truecolor', - TERM_PROGRAM: 'Orca', - FORCE_HYPERLINK: '1' - } as Record - }) - - ptyProcesses.set(id, ptyProcess) - ptyLoadGeneration.set(id, loadGeneration) - runtime?.onPtySpawned(id) - - ptyProcess.onData((data) => { - runtime?.onPtyData(id, data, Date.now()) - if (!mainWindow.isDestroyed()) { - mainWindow.webContents.send('pty:data', { id, data }) - } - }) - - ptyProcess.onExit(({ exitCode }) => { - ptyProcesses.delete(id) - ptyLoadGeneration.delete(id) - runtime?.onPtyExit(id, exitCode) - if (!mainWindow.isDestroyed()) { - mainWindow.webContents.send('pty:exit', { id, code: exitCode }) - } - }) - - return { id } - }) + ) ipcMain.on('pty:write', (_event, args: { id: string; data: string }) => { const proc = ptyProcesses.get(args.id) diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts new file mode 100644 index 000000000..466088f31 --- /dev/null +++ b/src/main/ipc/worktrees-windows.test.ts @@ -0,0 +1,251 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + handleMock, + removeHandlerMock, + listWorktreesMock, + addWorktreeMock, + removeWorktreeMock, + getGitUsernameMock, + getDefaultBaseRefMock, + getBranchConflictKindMock, + getPRForBranchMock, + getEffectiveHooksMock, + createSetupRunnerScriptMock, + shouldRunSetupForCreateMock, + runHookMock, + hasHooksFileMock, + loadHooksMock, + computeWorktreePathMock, + ensurePathWithinWorkspaceMock +} = vi.hoisted(() => ({ + handleMock: vi.fn(), + removeHandlerMock: vi.fn(), + listWorktreesMock: vi.fn(), + addWorktreeMock: vi.fn(), + removeWorktreeMock: vi.fn(), + getGitUsernameMock: vi.fn(), + getDefaultBaseRefMock: vi.fn(), + getBranchConflictKindMock: vi.fn(), + getPRForBranchMock: vi.fn(), + getEffectiveHooksMock: vi.fn(), + createSetupRunnerScriptMock: vi.fn(), + shouldRunSetupForCreateMock: vi.fn(), + runHookMock: vi.fn(), + hasHooksFileMock: vi.fn(), + loadHooksMock: vi.fn(), + computeWorktreePathMock: vi.fn(), + ensurePathWithinWorkspaceMock: vi.fn() +})) + +vi.mock('electron', () => ({ + ipcMain: { + handle: handleMock, + removeHandler: removeHandlerMock + } +})) + +vi.mock('../git/worktree', () => ({ + listWorktrees: listWorktreesMock, + addWorktree: addWorktreeMock, + removeWorktree: removeWorktreeMock +})) + +vi.mock('../git/repo', () => ({ + getGitUsername: getGitUsernameMock, + getDefaultBaseRef: getDefaultBaseRefMock, + getBranchConflictKind: getBranchConflictKindMock +})) + +vi.mock('../github/client', () => ({ + getPRForBranch: getPRForBranchMock +})) + +vi.mock('../hooks', () => ({ + createSetupRunnerScript: createSetupRunnerScriptMock, + getEffectiveHooks: getEffectiveHooksMock, + loadHooks: loadHooksMock, + runHook: runHookMock, + hasHooksFile: hasHooksFileMock, + shouldRunSetupForCreate: shouldRunSetupForCreateMock +})) + +vi.mock('./worktree-logic', async (importOriginal) => { + const actual = (await importOriginal()) as Record + return { + ...actual, + computeWorktreePath: computeWorktreePathMock, + ensurePathWithinWorkspace: ensurePathWithinWorkspaceMock + } +}) + +import { registerWorktreeHandlers } from './worktrees' + +type HandlerMap = Record unknown> + +describe('registerWorktreeHandlers – Windows path handling', () => { + const handlers: HandlerMap = {} + const mainWindow = { + isDestroyed: () => false, + webContents: { + send: vi.fn() + } + } + const store = { + getRepos: vi.fn(), + getRepo: vi.fn(), + getSettings: vi.fn(), + getWorktreeMeta: vi.fn(), + setWorktreeMeta: vi.fn(), + removeWorktreeMeta: vi.fn() + } + + beforeEach(() => { + handleMock.mockReset() + removeHandlerMock.mockReset() + listWorktreesMock.mockReset() + addWorktreeMock.mockReset() + removeWorktreeMock.mockReset() + getGitUsernameMock.mockReset() + getDefaultBaseRefMock.mockReset() + getBranchConflictKindMock.mockReset() + getPRForBranchMock.mockReset() + getEffectiveHooksMock.mockReset() + createSetupRunnerScriptMock.mockReset() + shouldRunSetupForCreateMock.mockReset() + runHookMock.mockReset() + hasHooksFileMock.mockReset() + loadHooksMock.mockReset() + computeWorktreePathMock.mockReset() + ensurePathWithinWorkspaceMock.mockReset() + mainWindow.webContents.send.mockReset() + store.getRepos.mockReset() + store.getRepo.mockReset() + store.getSettings.mockReset() + store.getWorktreeMeta.mockReset() + store.setWorktreeMeta.mockReset() + store.removeWorktreeMeta.mockReset() + + for (const key of Object.keys(handlers)) { + delete handlers[key] + } + + handleMock.mockImplementation((channel, handler) => { + handlers[channel] = handler + }) + + store.getRepo.mockReturnValue({ + id: 'repo-1', + path: 'C:\\repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + worktreeBaseRef: null + }) + store.getSettings.mockReturnValue({ + branchPrefix: 'none', + nestWorkspaces: false, + workspaceDir: 'C:\\workspaces' + }) + store.getWorktreeMeta.mockReturnValue(undefined) + store.setWorktreeMeta.mockReturnValue({}) + getGitUsernameMock.mockReturnValue('') + getDefaultBaseRefMock.mockReturnValue('origin/main') + getBranchConflictKindMock.mockResolvedValue(null) + getPRForBranchMock.mockResolvedValue(null) + getEffectiveHooksMock.mockReturnValue(null) + shouldRunSetupForCreateMock.mockReturnValue(false) + computeWorktreePathMock.mockReturnValue('C:\\workspaces\\improve-dashboard') + ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\improve-dashboard') + listWorktreesMock.mockResolvedValue([]) + + registerWorktreeHandlers(mainWindow as never, store as never) + }) + + it('accepts a newly created Windows worktree when git lists the same path with different separators', async () => { + listWorktreesMock.mockResolvedValue([ + { + path: 'C:/workspaces/improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + + const result = await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard' + }) + + expect(addWorktreeMock).toHaveBeenCalledWith( + 'C:\\repo', + 'C:\\workspaces\\improve-dashboard', + 'improve-dashboard', + 'origin/main' + ) + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::C:/workspaces/improve-dashboard', + expect.objectContaining({ + lastActivityAt: expect.any(Number) + }) + ) + expect(result).toMatchObject({ + worktree: expect.objectContaining({ + id: 'repo-1::C:/workspaces/improve-dashboard', + path: 'C:/workspaces/improve-dashboard', + branch: 'refs/heads/improve-dashboard' + }) + }) + }) + + it('preserves create-time metadata on the next list when Windows path formatting differs', async () => { + listWorktreesMock + .mockResolvedValueOnce([ + { + path: 'C:/workspaces/improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + .mockResolvedValueOnce([ + { + path: 'C:/workspaces/improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + store.setWorktreeMeta.mockReturnValue({ + lastActivityAt: 123, + displayName: 'Improve Dashboard' + }) + store.getWorktreeMeta.mockImplementation((worktreeId: string) => + worktreeId === 'repo-1::C:/workspaces/improve-dashboard' + ? { + lastActivityAt: 123, + displayName: 'Improve Dashboard' + } + : undefined + ) + + await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'Improve Dashboard' + }) + const listed = await handlers['worktrees:list'](null, { + repoId: 'repo-1' + }) + + expect(listed).toMatchObject([ + { + id: 'repo-1::C:/workspaces/improve-dashboard', + displayName: 'Improve Dashboard', + lastActivityAt: 123 + } + ]) + }) +}) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 9fab40a29..890b28a5b 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -11,6 +11,8 @@ const { getBranchConflictKindMock, getPRForBranchMock, getEffectiveHooksMock, + createSetupRunnerScriptMock, + shouldRunSetupForCreateMock, runHookMock, hasHooksFileMock, loadHooksMock, @@ -27,6 +29,8 @@ const { getBranchConflictKindMock: vi.fn(), getPRForBranchMock: vi.fn(), getEffectiveHooksMock: vi.fn(), + createSetupRunnerScriptMock: vi.fn(), + shouldRunSetupForCreateMock: vi.fn(), runHookMock: vi.fn(), hasHooksFileMock: vi.fn(), loadHooksMock: vi.fn(), @@ -58,10 +62,12 @@ vi.mock('../github/client', () => ({ })) vi.mock('../hooks', () => ({ + createSetupRunnerScript: createSetupRunnerScriptMock, getEffectiveHooks: getEffectiveHooksMock, loadHooks: loadHooksMock, runHook: runHookMock, - hasHooksFile: hasHooksFileMock + hasHooksFile: hasHooksFileMock, + shouldRunSetupForCreate: shouldRunSetupForCreateMock })) vi.mock('./worktree-logic', async (importOriginal) => { @@ -105,6 +111,8 @@ describe('registerWorktreeHandlers', () => { getBranchConflictKindMock.mockReset() getPRForBranchMock.mockReset() getEffectiveHooksMock.mockReset() + createSetupRunnerScriptMock.mockReset() + shouldRunSetupForCreateMock.mockReset() runHookMock.mockReset() hasHooksFileMock.mockReset() loadHooksMock.mockReset() @@ -146,6 +154,14 @@ describe('registerWorktreeHandlers', () => { getBranchConflictKindMock.mockResolvedValue(null) getPRForBranchMock.mockResolvedValue(null) getEffectiveHooksMock.mockReturnValue(null) + shouldRunSetupForCreateMock.mockReturnValue(false) + createSetupRunnerScriptMock.mockReturnValue({ + runnerScriptPath: '/workspace/repo/.git/orca/setup-runner.sh', + envVars: { + ORCA_ROOT_PATH: '/workspace/repo', + ORCA_WORKTREE_PATH: '/workspace/improve-dashboard' + } + }) computeWorktreePathMock.mockImplementation( ( sanitizedName: string, @@ -153,7 +169,11 @@ describe('registerWorktreeHandlers', () => { settings: { nestWorkspaces: boolean; workspaceDir: string } ) => { if (settings.nestWorkspaces) { - const repoName = repoPath.split(/[\\/]/).at(-1)?.replace(/\.git$/, '') ?? 'repo' + const repoName = + repoPath + .split(/[\\/]/) + .at(-1) + ?.replace(/\.git$/, '') ?? 'repo' return `${settings.workspaceDir}/${repoName}/${sanitizedName}` } return `${settings.workspaceDir}/${sanitizedName}` @@ -204,118 +224,107 @@ describe('registerWorktreeHandlers', () => { expect(addWorktreeMock).not.toHaveBeenCalled() }) - it('accepts a newly created Windows worktree when git lists the same path with different separators', async () => { - store.getRepo.mockReturnValue({ - id: 'repo-1', - path: 'C:\\repo', - displayName: 'repo', - badgeColor: '#000', - addedAt: 0, - worktreeBaseRef: null - }) - store.getSettings.mockReturnValue({ - branchPrefix: 'none', - nestWorkspaces: false, - workspaceDir: 'C:\\workspaces' - }) - computeWorktreePathMock.mockReturnValue('C:\\workspaces\\improve-dashboard') - ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\improve-dashboard') + it('returns a setup launch payload when setup should run', async () => { listWorktreesMock.mockResolvedValue([ { - path: 'C:/workspaces/improve-dashboard', + path: '/workspace/improve-dashboard', head: 'abc123', - branch: 'refs/heads/improve-dashboard', + branch: 'improve-dashboard', isBare: false, isMainWorktree: false } ]) + getEffectiveHooksMock.mockReturnValue({ + scripts: { + setup: 'pnpm worktree:setup' + } + }) + shouldRunSetupForCreateMock.mockReturnValue(true) const result = await handlers['worktrees:create'](null, { repoId: 'repo-1', - name: 'improve-dashboard' + name: 'improve-dashboard', + setupDecision: 'run' }) - expect(addWorktreeMock).toHaveBeenCalledWith( - 'C:\\repo', - 'C:\\workspaces\\improve-dashboard', - 'improve-dashboard', - 'origin/main' + expect(createSetupRunnerScriptMock).toHaveBeenCalledWith( + expect.objectContaining({ id: 'repo-1' }), + '/workspace/improve-dashboard', + 'pnpm worktree:setup' ) - expect(store.setWorktreeMeta).toHaveBeenCalledWith( - 'repo-1::C:/workspaces/improve-dashboard', - expect.objectContaining({ - lastActivityAt: expect.any(Number) - }) - ) - expect(result).toMatchObject({ - id: 'repo-1::C:/workspaces/improve-dashboard', - path: 'C:/workspaces/improve-dashboard', - branch: 'refs/heads/improve-dashboard' + expect(result).toEqual({ + worktree: expect.objectContaining({ + repoId: 'repo-1', + path: '/workspace/improve-dashboard', + branch: 'improve-dashboard' + }), + setup: { + runnerScriptPath: '/workspace/repo/.git/orca/setup-runner.sh', + envVars: { + ORCA_ROOT_PATH: '/workspace/repo', + ORCA_WORKTREE_PATH: '/workspace/improve-dashboard' + } + } }) }) - it('preserves create-time metadata on the next list when Windows path formatting differs', async () => { - store.getRepo.mockReturnValue({ - id: 'repo-1', - path: 'C:\\repo', - displayName: 'repo', - badgeColor: '#000', - addedAt: 0, - worktreeBaseRef: null - }) - store.getSettings.mockReturnValue({ - branchPrefix: 'none', - nestWorkspaces: false, - workspaceDir: 'C:\\workspaces' - }) - computeWorktreePathMock.mockReturnValue('C:\\workspaces\\improve-dashboard') - ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\improve-dashboard') - listWorktreesMock - .mockResolvedValueOnce([ - { - path: 'C:/workspaces/improve-dashboard', - head: 'abc123', - branch: 'refs/heads/improve-dashboard', - isBare: false, - isMainWorktree: false - } - ]) - .mockResolvedValueOnce([ - { - path: 'C:/workspaces/improve-dashboard', - head: 'abc123', - branch: 'refs/heads/improve-dashboard', - isBare: false, - isMainWorktree: false - } - ]) - store.setWorktreeMeta.mockReturnValue({ - lastActivityAt: 123, - displayName: 'Improve Dashboard' - }) - store.getWorktreeMeta.mockImplementation((worktreeId: string) => - worktreeId === 'repo-1::C:/workspaces/improve-dashboard' - ? { - lastActivityAt: 123, - displayName: 'Improve Dashboard' - } - : undefined - ) - - await handlers['worktrees:create'](null, { - repoId: 'repo-1', - name: 'Improve Dashboard' - }) - const listed = await handlers['worktrees:list'](null, { - repoId: 'repo-1' - }) - - expect(listed).toMatchObject([ + it('still returns the created worktree when setup runner generation fails', async () => { + listWorktreesMock.mockResolvedValue([ { - id: 'repo-1::C:/workspaces/improve-dashboard', - displayName: 'Improve Dashboard', - lastActivityAt: 123 + path: '/workspace/improve-dashboard', + head: 'abc123', + branch: 'improve-dashboard', + isBare: false, + isMainWorktree: false } ]) + getEffectiveHooksMock.mockReturnValue({ + scripts: { + setup: 'pnpm worktree:setup' + } + }) + shouldRunSetupForCreateMock.mockReturnValue(true) + createSetupRunnerScriptMock.mockImplementation(() => { + throw new Error('disk full') + }) + + const result = await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard', + setupDecision: 'run' + }) + + expect(result).toEqual({ + worktree: expect.objectContaining({ + repoId: 'repo-1', + path: '/workspace/improve-dashboard', + branch: 'improve-dashboard' + }) + }) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { + repoId: 'repo-1' + }) + }) + + it('rejects ask-policy creates before mutating git state when setup decision is missing', async () => { + getEffectiveHooksMock.mockReturnValue({ + scripts: { + setup: 'pnpm worktree:setup' + } + }) + shouldRunSetupForCreateMock.mockImplementation(() => { + throw new Error('Setup decision required for this repository') + }) + + await expect( + handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard' + }) + ).rejects.toThrow('Setup decision required for this repository') + + expect(addWorktreeMock).not.toHaveBeenCalled() + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(createSetupRunnerScriptMock).not.toHaveBeenCalled() }) }) diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 797090312..380a01b96 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -3,11 +3,23 @@ import { ipcMain } from 'electron' import { execFileSync } from 'child_process' import { rm } from 'fs/promises' import type { Store } from '../persistence' -import type { Worktree, WorktreeMeta } from '../../shared/types' +import type { + CreateWorktreeArgs, + CreateWorktreeResult, + Worktree, + WorktreeMeta +} from '../../shared/types' import { getPRForBranch } from '../github/client' import { listWorktrees, addWorktree, removeWorktree } from '../git/worktree' import { getGitUsername, getDefaultBaseRef, getBranchConflictKind } from '../git/repo' -import { getEffectiveHooks, loadHooks, runHook, hasHooksFile } from '../hooks' +import { + createSetupRunnerScript, + getEffectiveHooks, + loadHooks, + runHook, + hasHooksFile, + shouldRunSetupForCreate +} from '../hooks' import { sanitizeWorktreeName, computeBranchName, @@ -63,7 +75,7 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store ipcMain.handle( 'worktrees:create', - async (_event, args: { repoId: string; name: string; baseBranch?: string }) => { + async (_event, args: CreateWorktreeArgs): Promise => { const repo = store.getRepo(args.repoId) if (!repo) { throw new Error(`Repo not found: ${args.repoId}`) @@ -107,6 +119,13 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store // Determine base branch const baseBranch = args.baseBranch || repo.worktreeBaseRef || getDefaultBaseRef(repo.path) + const setupScript = getEffectiveHooks(repo)?.scripts.setup + // Why: `ask` is a pre-create choice gate, not a post-create side effect. + // Resolve it before mutating git state so missing UI input cannot strand + // a real worktree on disk while the renderer reports "create failed". + const shouldLaunchSetup = setupScript + ? shouldRunSetupForCreate(repo, args.setupDecision) + : false // Fetch latest from remote so the worktree starts with up-to-date content const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin' @@ -142,18 +161,29 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store const meta = store.setWorktreeMeta(worktreeId, metaUpdates) const worktree = mergeWorktree(repo.id, created, meta) - // Run setup hook asynchronously (don't block the UI) - const hooks = getEffectiveHooks(repo) - if (hooks?.scripts.setup) { - runHook('setup', worktreePath, repo).then((result) => { - if (!result.success) { - console.error(`[hooks] setup hook failed for ${worktreePath}:`, result.output) - } - }) + let setup: CreateWorktreeResult['setup'] + if (setupScript && shouldLaunchSetup) { + try { + // Why: setup now runs in a visible terminal owned by the renderer so users + // can inspect failures, answer prompts, and rerun it. The main process only + // resolves policy and writes the runner script; it must not execute setup + // itself anymore or we would reintroduce the hidden background-hook behavior. + // + // Why: the git worktree already exists at this point. If runner generation + // fails, surfacing the error as a hard create failure would lie to the UI + // about the underlying git state and strand a real worktree on disk. + // Degrade to "created without setup launch" instead. + setup = createSetupRunnerScript(repo, worktreePath, setupScript) + } catch (error) { + console.error(`[hooks] Failed to prepare setup runner for ${worktreePath}:`, error) + } } notifyWorktreesChanged(mainWindow, repo.id) - return worktree + return { + worktree, + ...(setup ? { setup } : {}) + } } ) diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 0dfbad682..b6c9b220d 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -271,6 +271,22 @@ describe('Store', () => { expect(persisted.repos[0].id).toBe('r1') }) + it('flush remains safe when a debounced save is also pending', async () => { + vi.useFakeTimers() + try { + const store = await createStore() + store.addRepo(makeRepo()) + store.flush() + vi.advanceTimersByTime(300) + + const persisted = readDataFile() as { repos: Repo[] } + expect(persisted.repos).toHaveLength(1) + expect(persisted.repos[0].id).toBe('r1') + } finally { + vi.useRealTimers() + } + }) + // ── 11. Debounced save ───────────────────────────────────────────── it('debounced save writes data after the delay', async () => { diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 9ace5f8a3..e5faa0d23 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -64,19 +64,28 @@ export class Store { this.writeTimer = setTimeout(() => { this.writeTimer = null try { - const dir = dirname(DATA_FILE) - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } - const tmpFile = `${DATA_FILE}.tmp` - writeFileSync(tmpFile, JSON.stringify(this.state, null, 2), 'utf-8') - renameSync(tmpFile, DATA_FILE) + this.writeToDisk() } catch (err) { console.error('[persistence] Failed to write state:', err) } }, 300) } + private writeToDisk(): void { + const dir = dirname(DATA_FILE) + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }) + } + // Why: synchronous flushes can race the debounced writer during shutdown or + // beforeunload persistence. A shared `.tmp` path lets one rename steal the + // temp file from the other, which surfaces as ENOENT even though the final + // state may already be on disk. Use a unique temp file per write so atomic + // replaces remain race-safe across platforms. + const tmpFile = `${DATA_FILE}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` + writeFileSync(tmpFile, JSON.stringify(this.state, null, 2), 'utf-8') + renameSync(tmpFile, DATA_FILE) + } + // ── Repos ────────────────────────────────────────────────────────── getRepos(): Repo[] { @@ -230,13 +239,7 @@ export class Store { this.writeTimer = null } try { - const dir = dirname(DATA_FILE) - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } - const tmpFile = `${DATA_FILE}.tmp` - writeFileSync(tmpFile, JSON.stringify(this.state, null, 2), 'utf-8') - renameSync(tmpFile, DATA_FILE) + this.writeToDisk() } catch (err) { console.error('[persistence] Failed to flush state:', err) } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index a75f6f2ae..962148556 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -2,10 +2,15 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import type { WorktreeMeta } from '../../shared/types' import { addWorktree, listWorktrees } from '../git/worktree' +import { createSetupRunnerScript, getEffectiveHooks, runHook } from '../hooks' import { OrcaRuntimeService } from './orca-runtime' -const { MOCK_GIT_WORKTREES, addWorktreeMock, computeWorktreePathMock, ensurePathWithinWorkspaceMock } = - vi.hoisted(() => ({ +const { + MOCK_GIT_WORKTREES, + addWorktreeMock, + computeWorktreePathMock, + ensurePathWithinWorkspaceMock +} = vi.hoisted(() => ({ MOCK_GIT_WORKTREES: [ { path: '/tmp/worktree-a', @@ -25,6 +30,12 @@ vi.mock('../git/worktree', () => ({ addWorktree: addWorktreeMock })) +vi.mock('../hooks', () => ({ + createSetupRunnerScript: vi.fn(), + getEffectiveHooks: vi.fn().mockReturnValue(null), + runHook: vi.fn().mockResolvedValue({ success: true, output: '' }) +})) + vi.mock('../ipc/worktree-logic', async (importOriginal) => { const actual = (await importOriginal()) as Record return { @@ -37,16 +48,30 @@ vi.mock('../ipc/worktree-logic', async (importOriginal) => { afterEach(() => { vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES) vi.mocked(addWorktree).mockReset() + vi.mocked(createSetupRunnerScript).mockReset() + vi.mocked(getEffectiveHooks).mockReset() + vi.mocked(runHook).mockReset() + vi.mocked(getEffectiveHooks).mockReturnValue(null) computeWorktreePathMock.mockReset() ensurePathWithinWorkspaceMock.mockReset() }) +const TEST_WINDOW_ID = 1 +const TEST_REPO_ID = 'repo-1' +const TEST_REPO_PATH = '/tmp/repo' +const TEST_WORKTREE_PATH = '/tmp/worktree-a' +const TEST_WORKTREE_ID = `${TEST_REPO_ID}::${TEST_WORKTREE_PATH}` + +function createRuntime(): OrcaRuntimeService { + return new OrcaRuntimeService(store) +} + const store = { getRepo: (id: string) => store.getRepos().find((repo) => repo.id === id), getRepos: () => [ { - id: 'repo-1', - path: '/tmp/repo', + id: TEST_REPO_ID, + path: TEST_REPO_PATH, displayName: 'repo', badgeColor: 'blue', addedAt: 1 @@ -59,7 +84,7 @@ const store = { ...updates }) as never, getAllWorktreeMeta: () => ({ - 'repo-1::/tmp/worktree-a': { + [TEST_WORKTREE_ID]: { displayName: 'foo', comment: '', linkedIssue: 123, @@ -73,7 +98,7 @@ const store = { getWorktreeMeta: (worktreeId: string) => store.getAllWorktreeMeta()[worktreeId], setWorktreeMeta: (_worktreeId: string, meta: Record) => ({ - ...store.getAllWorktreeMeta()['repo-1::/tmp/worktree-a'], + ...store.getAllWorktreeMeta()[TEST_WORKTREE_ID], ...meta }) as never, removeWorktreeMeta: () => {}, @@ -92,7 +117,11 @@ computeWorktreePathMock.mockImplementation( settings: { nestWorkspaces: boolean; workspaceDir: string } ) => { if (settings.nestWorkspaces) { - const repoName = repoPath.split(/[\\/]/).at(-1)?.replace(/\.git$/, '') ?? 'repo' + const repoName = + repoPath + .split(/[\\/]/) + .at(-1) + ?.replace(/\.git$/, '') ?? 'repo' return `${settings.workspaceDir}/${repoName}/${sanitizedName}` } return `${settings.workspaceDir}/${sanitizedName}` @@ -102,7 +131,7 @@ ensurePathWithinWorkspaceMock.mockImplementation((targetPath: string) => targetP describe('OrcaRuntimeService', () => { it('starts unavailable with no authoritative window', () => { - const runtime = new OrcaRuntimeService(store) + const runtime = createRuntime() expect(runtime.getStatus()).toMatchObject({ graphStatus: 'unavailable', @@ -113,20 +142,20 @@ describe('OrcaRuntimeService', () => { }) it('claims the first window as authoritative and ignores later windows', () => { - const runtime = new OrcaRuntimeService(store) + const runtime = createRuntime() - runtime.attachWindow(1) + runtime.attachWindow(TEST_WINDOW_ID) runtime.attachWindow(2) - expect(runtime.getStatus().authoritativeWindowId).toBe(1) + expect(runtime.getStatus().authoritativeWindowId).toBe(TEST_WINDOW_ID) }) it('bumps the epoch and enters reloading when the authoritative window reloads', () => { - const runtime = new OrcaRuntimeService(store) + const runtime = createRuntime() - runtime.attachWindow(1) - runtime.markGraphReady(1) - runtime.markRendererReloading(1) + runtime.attachWindow(TEST_WINDOW_ID) + runtime.markGraphReady(TEST_WINDOW_ID) + runtime.markRendererReloading(TEST_WINDOW_ID) expect(runtime.getStatus()).toMatchObject({ graphStatus: 'reloading', @@ -135,23 +164,23 @@ describe('OrcaRuntimeService', () => { }) it('can mark the graph ready for the authoritative window', () => { - const runtime = new OrcaRuntimeService(store) + const runtime = createRuntime() - runtime.attachWindow(1) - runtime.markGraphReady(1) - runtime.markRendererReloading(1) - runtime.markGraphReady(1) + runtime.attachWindow(TEST_WINDOW_ID) + runtime.markGraphReady(TEST_WINDOW_ID) + runtime.markRendererReloading(TEST_WINDOW_ID) + runtime.markGraphReady(TEST_WINDOW_ID) expect(runtime.getStatus().graphStatus).toBe('ready') }) it('drops back to unavailable and clears authority when the window disappears', () => { - const runtime = new OrcaRuntimeService(store) + const runtime = createRuntime() - runtime.attachWindow(1) - runtime.markGraphReady(1) - runtime.markRendererReloading(1) - runtime.markGraphUnavailable(1) + runtime.attachWindow(TEST_WINDOW_ID) + runtime.markGraphReady(TEST_WINDOW_ID) + runtime.markRendererReloading(TEST_WINDOW_ID) + runtime.markGraphUnavailable(TEST_WINDOW_ID) expect(runtime.getStatus()).toMatchObject({ graphStatus: 'unavailable', @@ -161,10 +190,10 @@ describe('OrcaRuntimeService', () => { }) it('stays unavailable during initial loads before a graph is published', () => { - const runtime = new OrcaRuntimeService(store) + const runtime = createRuntime() - runtime.attachWindow(1) - runtime.markRendererReloading(1) + runtime.attachWindow(TEST_WINDOW_ID) + runtime.markRendererReloading(TEST_WINDOW_ID) expect(runtime.getStatus()).toMatchObject({ graphStatus: 'unavailable', @@ -542,6 +571,68 @@ describe('OrcaRuntimeService', () => { await expect(runtime.searchRepoRefs('id:repo-1', 'main', -5)).rejects.toThrow('invalid_limit') }) + it('returns a setup launch payload for CLI-created worktrees when orca.yaml defines setup', async () => { + const runtime = new OrcaRuntimeService(store) + const activateWorktree = vi.fn() + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree + }) + runtime.attachWindow(1) + + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-hook-test') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-hook-test') + vi.mocked(getEffectiveHooks).mockReturnValue({ + scripts: { + setup: 'pnpm worktree:setup' + } + }) + vi.mocked(createSetupRunnerScript).mockReturnValue({ + runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh', + envVars: { + ORCA_ROOT_PATH: '/tmp/repo', + ORCA_WORKTREE_PATH: '/tmp/workspaces/runtime-hook-test' + } + }) + vi.mocked(listWorktrees).mockResolvedValueOnce([ + { + path: '/tmp/workspaces/runtime-hook-test', + head: 'def', + branch: 'runtime-hook-test', + isBare: false, + isMainWorktree: false + } + ]) + + const result = await runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'runtime-hook-test' + }) + + expect(createSetupRunnerScript).toHaveBeenCalledWith( + expect.objectContaining({ id: 'repo-1', path: '/tmp/repo' }), + '/tmp/workspaces/runtime-hook-test', + 'pnpm worktree:setup' + ) + expect(runHook).not.toHaveBeenCalled() + expect(result).toEqual({ + worktree: expect.objectContaining({ + repoId: 'repo-1', + path: '/tmp/workspaces/runtime-hook-test', + branch: 'runtime-hook-test' + }), + setup: { + runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh', + envVars: { + ORCA_ROOT_PATH: '/tmp/repo', + ORCA_WORKTREE_PATH: '/tmp/workspaces/runtime-hook-test' + } + } + }) + expect(activateWorktree).toHaveBeenCalledWith('repo-1', expect.any(String), result.setup) + }) + it('preserves create-time metadata on later runtime listings when Windows path formatting differs', async () => { const metaById: Record = {} const runtimeStore = { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 9940e736b..83ad8ff91 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -4,7 +4,7 @@ import { execFileSync } from 'child_process' import { randomUUID } from 'crypto' import { rm } from 'fs/promises' -import type { Repo } from '../../shared/types' +import type { CreateWorktreeResult, Repo } from '../../shared/types' import type { RuntimeGraphStatus, RuntimeRepoSearchRefs, @@ -26,7 +26,7 @@ import { listWorktrees } from '../git/worktree' import { getPRForBranch } from '../github/client' import { getGitUsername, getDefaultBaseRef, getBranchConflictKind } from '../git/repo' import { addWorktree, removeWorktree } from '../git/worktree' -import { getEffectiveHooks, runHook } from '../hooks' +import { createSetupRunnerScript, getEffectiveHooks, runHook } from '../hooks' import { REPO_COLORS } from '../../shared/constants' import { isGitRepo, getRepoName, searchBaseRefs } from '../git/repo' import type { Store } from '../persistence' @@ -79,7 +79,7 @@ type RuntimePtyController = { type RuntimeNotifier = { worktreesChanged(repoId: string): void reposChanged(): void - activateWorktree(repoId: string, worktreeId: string): void + activateWorktree(repoId: string, worktreeId: string, setup?: CreateWorktreeResult['setup']): void } type TerminalHandleRecord = { @@ -543,7 +543,7 @@ export class OrcaRuntimeService { baseBranch?: string linkedIssue?: number | null comment?: string - }) { + }): Promise { if (!this.store) { throw new Error('runtime_unavailable') } @@ -607,13 +607,29 @@ export class OrcaRuntimeService { }) const worktree = mergeWorktree(repo.id, created, meta) + let setup: CreateWorktreeResult['setup'] const hooks = getEffectiveHooks(repo) if (hooks?.scripts.setup) { - void runHook('setup', worktreePath, repo).then((result) => { - if (!result.success) { - console.error(`[hooks] setup hook failed for ${worktreePath}:`, result.output) + if (this.authoritativeWindowId !== null) { + try { + // Why: CLI-created worktrees must use the same runner-script path as the + // renderer create flow so repo-committed `orca.yaml` setup hooks run in + // the visible first terminal instead of a hidden background shell with + // different failure and prompt behavior. + setup = createSetupRunnerScript(repo, worktreePath, hooks.scripts.setup) + } catch (error) { + // Why: the git worktree is already real at this point. If runner + // generation fails, keep creation successful and surface the problem in + // logs rather than pretending the worktree was never created. + console.error(`[hooks] Failed to prepare setup runner for ${worktreePath}:`, error) } - }) + } else { + void runHook('setup', worktreePath, repo).then((result) => { + if (!result.success) { + console.error(`[hooks] setup hook failed for ${worktreePath}:`, result.output) + } + }) + } } this.notifier?.worktreesChanged(repo.id) @@ -621,9 +637,12 @@ export class OrcaRuntimeService { // renderer-side consequence of activating a worktree. CLI-created // worktrees must trigger that same activation path or they will exist on // disk without becoming the active workspace in the UI. - this.notifier?.activateWorktree(repo.id, worktree.id) + this.notifier?.activateWorktree(repo.id, worktree.id, setup) this.invalidateResolvedWorktreeCache() - return worktree + return { + worktree, + ...(setup ? { setup } : {}) + } } async updateManagedWorktreeMeta( diff --git a/src/main/runtime/runtime-rpc.ts b/src/main/runtime/runtime-rpc.ts index baa4ecc8f..db2f893e9 100644 --- a/src/main/runtime/runtime-rpc.ts +++ b/src/main/runtime/runtime-rpc.ts @@ -580,7 +580,7 @@ export class OrcaRuntimeRpcServer { return { id: request.id, ok: true, - result: { worktree: result }, + result, _meta: { runtimeId: this.runtime.getRuntimeId() } diff --git a/src/main/window/attach-main-window-services.test.ts b/src/main/window/attach-main-window-services.test.ts index 2883010f6..48a26365e 100644 --- a/src/main/window/attach-main-window-services.test.ts +++ b/src/main/window/attach-main-window-services.test.ts @@ -122,17 +122,40 @@ describe('attachMainWindowServices', () => { const notifier = runtime.setNotifier.mock.calls[0][0] as { worktreesChanged: (repoId: string) => void reposChanged: () => void - activateWorktree: (repoId: string, worktreeId: string) => void + activateWorktree: ( + repoId: string, + worktreeId: string, + setup?: { runnerScriptPath: string; envVars: Record } + ) => void } notifier.worktreesChanged('repo-1') notifier.reposChanged() - notifier.activateWorktree('repo-1', 'wt-1') + notifier.activateWorktree('repo-1', 'wt-1', { + runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh', + envVars: { + ORCA_ROOT_PATH: '/tmp/repo', + ORCA_WORKTREE_PATH: '/tmp/worktrees/wt-1' + } + }) expect(sendMock.mock.calls).toEqual([ ['worktrees:changed', { repoId: 'repo-1' }], ['repos:changed'], - ['ui:activateWorktree', { repoId: 'repo-1', worktreeId: 'wt-1' }] + [ + 'ui:activateWorktree', + { + repoId: 'repo-1', + worktreeId: 'wt-1', + setup: { + runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh', + envVars: { + ORCA_ROOT_PATH: '/tmp/repo', + ORCA_WORKTREE_PATH: '/tmp/worktrees/wt-1' + } + } + } + ] ]) }) }) diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 0ffb75ec9..93638c416 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -1,6 +1,7 @@ import { app, clipboard, ipcMain } from 'electron' import type { BrowserWindow } from 'electron' import type { Store } from '../persistence' +import type { CreateWorktreeResult } from '../../shared/types' import { registerRepoHandlers } from '../ipc/repos' import { registerWorktreeHandlers } from '../ipc/worktrees' import { registerPtyHandlers } from '../ipc/pty' @@ -55,9 +56,9 @@ function registerRuntimeWindowLifecycle( mainWindow.webContents.send('repos:changed') } }, - activateWorktree: (repoId, worktreeId) => { + activateWorktree: (repoId, worktreeId, setup?: CreateWorktreeResult['setup']) => { if (!mainWindow.isDestroyed()) { - mainWindow.webContents.send('ui:activateWorktree', { repoId, worktreeId }) + mainWindow.webContents.send('ui:activateWorktree', { repoId, worktreeId, setup }) } } }) diff --git a/src/preload/index.d.ts b/src/preload/index.d.ts index db501d4c8..38f5a5349 100644 --- a/src/preload/index.d.ts +++ b/src/preload/index.d.ts @@ -4,6 +4,8 @@ import type { Repo, Worktree, WorktreeMeta, + CreateWorktreeArgs, + CreateWorktreeResult, PRInfo, PRCheckDetail, IssueInfo, @@ -11,6 +13,7 @@ import type { OrcaHooks, PersistedUIState, WorkspaceSessionState, + WorktreeSetupLaunch, UpdateStatus, DirEntry, GitBranchCompareResult, @@ -40,14 +43,19 @@ type ReposApi = { type WorktreesApi = { list: (args: { repoId: string }) => Promise listAll: () => Promise - create: (args: { repoId: string; name: string; baseBranch?: string }) => Promise + create: (args: CreateWorktreeArgs) => Promise remove: (args: { worktreeId: string; force?: boolean }) => Promise updateMeta: (args: { worktreeId: string; updates: Partial }) => Promise onChanged: (callback: (data: { repoId: string }) => void) => () => void } type PtyApi = { - spawn: (opts: { cols: number; rows: number; cwd?: string }) => Promise<{ id: string }> + spawn: (opts: { + cols: number + rows: number + cwd?: string + env?: Record + }) => Promise<{ id: string }> write: (id: string, data: string) => void resize: (id: string, cols: number, rows: number) => void kill: (id: string) => Promise @@ -130,7 +138,7 @@ type UIApi = { set: (args: Partial) => Promise onOpenSettings: (callback: () => void) => () => void onActivateWorktree: ( - callback: (data: { repoId: string; worktreeId: string }) => void + callback: (data: { repoId: string; worktreeId: string; setup?: WorktreeSetupLaunch }) => void ) => () => void onTerminalZoom: (callback: (direction: 'in' | 'out' | 'reset') => void) => () => void readClipboardText: () => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index 0f6812ce4..d805e4ad7 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1,3 +1,6 @@ +/* eslint-disable max-lines -- Why: the preload bridge is the audited contract between +renderer and Electron. Keeping the IPC surface co-located in one file makes security +review and type drift checks easier than scattering these bindings across modules. */ import { contextBridge, ipcRenderer, webFrame, webUtils } from 'electron' import { electronAPI } from '@electron-toolkit/preload' import type { CliInstallStatus } from '../shared/cli-install-types' @@ -86,8 +89,12 @@ const api = { listAll: (): Promise => ipcRenderer.invoke('worktrees:listAll'), - create: (args: { repoId: string; name: string; baseBranch?: string }): Promise => - ipcRenderer.invoke('worktrees:create', args), + create: (args: { + repoId: string + name: string + baseBranch?: string + setupDecision?: 'inherit' | 'run' | 'skip' + }): Promise => ipcRenderer.invoke('worktrees:create', args), remove: (args: { worktreeId: string; force?: boolean }): Promise => ipcRenderer.invoke('worktrees:remove', args), @@ -106,8 +113,12 @@ const api = { }, pty: { - spawn: (opts: { cols: number; rows: number; cwd?: string }): Promise<{ id: string }> => - ipcRenderer.invoke('pty:spawn', opts), + spawn: (opts: { + cols: number + rows: number + cwd?: string + env?: Record + }): Promise<{ id: string }> => ipcRenderer.invoke('pty:spawn', opts), write: (id: string, data: string): void => { ipcRenderer.send('pty:write', { id, data }) @@ -310,11 +321,19 @@ const api = { return () => ipcRenderer.removeListener('ui:openSettings', listener) }, onActivateWorktree: ( - callback: (data: { repoId: string; worktreeId: string }) => void + callback: (data: { + repoId: string + worktreeId: string + setup?: { runnerScriptPath: string; envVars: Record } + }) => void ): (() => void) => { const listener = ( _event: Electron.IpcRendererEvent, - data: { repoId: string; worktreeId: string } + data: { + repoId: string + worktreeId: string + setup?: { runnerScriptPath: string; envVars: Record } + } ) => callback(data) ipcRenderer.on('ui:activateWorktree', listener) return () => ipcRenderer.removeListener('ui:activateWorktree', listener) diff --git a/src/renderer/src/components/settings/BaseRefPicker.tsx b/src/renderer/src/components/settings/BaseRefPicker.tsx index 5a81e56ad..ce2f0ea8c 100644 --- a/src/renderer/src/components/settings/BaseRefPicker.tsx +++ b/src/renderer/src/components/settings/BaseRefPicker.tsx @@ -90,7 +90,7 @@ export function BaseRefPicker({ const effectiveBaseRef = currentBaseRef ?? defaultBaseRef return ( -
+
{effectiveBaseRef}
diff --git a/src/renderer/src/components/settings/HookEditor.tsx b/src/renderer/src/components/settings/HookEditor.tsx deleted file mode 100644 index 13c219aa9..000000000 --- a/src/renderer/src/components/settings/HookEditor.tsx +++ /dev/null @@ -1,89 +0,0 @@ -import type { OrcaHooks, Repo } from '../../../../shared/types' -import { Label } from '../ui/label' -import type { HookName } from './SettingsConstants' - -export function HookEditor({ - hookName, - repo, - yamlHooks, - onScriptChange -}: { - hookName: HookName - repo: Repo - yamlHooks: OrcaHooks | null - onScriptChange: (script: string) => void -}): React.JSX.Element { - const uiScript = repo.hookSettings?.scripts[hookName] ?? '' - const yamlScript = yamlHooks?.scripts[hookName] - const effectiveSource = - repo.hookSettings?.mode === 'auto' && yamlScript ? 'yaml' : uiScript.trim() ? 'ui' : 'none' - - return ( -
-
-
-
{hookName}
-

- {hookName === 'setup' - ? 'Runs after a worktree is created.' - : 'Runs before a worktree is archived.'} -

-
- - - {effectiveSource === 'yaml' - ? 'Honoring YAML' - : effectiveSource === 'ui' - ? 'Using UI' - : 'Inactive'} - -
- - {yamlScript && ( -
-
- - Read-only from `orca.yaml` -
- 
-            {yamlScript}
-
-
- )} - -
-
- - - {repo.hookSettings?.mode === 'auto' && yamlScript - ? 'Stored as fallback until you switch to override.' - : 'Editable script stored with this repo.'} - -
-