From c846eb0aeaaca6ca49a6ba3b5b5cf16beaba888d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 17:09:46 -0300 Subject: [PATCH 01/13] docs(specs): plan worktree post-create hook feature (WPC-01..24) Spec, design and tasks for a repo-declared command that runs in each new worktree. Records AD-013: repo-local .app/config.json, keep-the-worktree failure semantics, all three create paths, inline dialog feedback, and the withPostCreateHook decorator architecture. Co-Authored-By: Claude Opus 5 (1M context) --- .specs/STATE.md | 1 + .../worktree-post-create-hook/design.md | 129 +++++++ .../worktree-post-create-hook/spec.md | 261 +++++++++++++ .../worktree-post-create-hook/tasks.md | 350 ++++++++++++++++++ 4 files changed, 741 insertions(+) create mode 100644 .specs/features/worktree-post-create-hook/design.md create mode 100644 .specs/features/worktree-post-create-hook/spec.md create mode 100644 .specs/features/worktree-post-create-hook/tasks.md diff --git a/.specs/STATE.md b/.specs/STATE.md index 0607a92..1afa38a 100644 --- a/.specs/STATE.md +++ b/.specs/STATE.md @@ -19,6 +19,7 @@ Handoff snapshot. | AD-011 | 2026-07-06 | ~~**WF5 (Workflows UI) scope pinned via 3 owner decisions**~~ **(decisions 1 & 2 AMENDED by AD-012; decision 3 stands):** (1) **Run state is live-stream only** — the view accumulates `workflow:*` events in an always-App-mounted `useWorkflowRuns` hook (survives direction switches); NO read channel for persisted/past runs (v2). (2) **A failed run shows only its `failed` status** in the UI — `error`/`stdout`/`code` are captured server-side but not broadcast (deferred). (3) **"New workflow" = scaffold + reveal** via a NEW `workflows:scaffold` channel; the created folder is revealed **main-side** with `shell.showItemInFolder` (no editor coupling). **Architecture:** the fold logic is a pure, unit-tested `workflow-run-view.ts` (like `tree-selection`); only `workflow-run-view` + `workflow-scaffold` carry unit tests, the rest (view, dialogs, hook wiring, handler) is hand-verified per project UI convention. 10 tasks / 3 phases (inline). | Owner chose live-only to match the PRD's v1-ephemeral posture with zero backend; failure-detail broadcast is cheap-but-deferred; scaffold+reveal avoids editor coupling. The always-mounted hook is required so a WF4 `workflow:focus-run` toast restores a run's full timeline from any direction. Spec/design/tasks: `.specs/features/workflows-ui/` (WF5-01..25). | | AD-009 | 2026-07-06 | **WF3 MERGED to `main` (PR #65).** Independent SDD eval (author≠judge, `spec-driven-eval`): **Final 0.98 — "Spec-complete"** (S=PASS, E recall/precision/justified ≈1.0, gates build/lint/unit green; live smoke owner-PASS 6/6). Two minor gaps merged as-is and **carried into WF4** (WF3-04 generic retry prompt; WF3-10 unasserted server reuse). **WF4 planning deferred to the next session.** | The two gaps are cheap polish on the same runner/`--resume` path WF4 already touches, so folding them into WF4 avoids a throwaway PR. Report: `.specs/features/workflows-agent-step/evaluations/P1-workflows-agent-step-20260706T141244Z.md`. | | AD-008 | 2026-07-03 | **WF3 (Structured agent step) scope pinned via 4 owner decisions:** (1) **Arm M (MCP) only** — one shared loopback HTTP MCP server, per-step bearer token = auth+routing, forced `emit_result`; Arm N (`--json-schema`) dropped. (2) **ajv** for payload validation (promotes `emit-result-schema` off the spike's minimal checker; `expect` stays a JSON Schema). (3) `ctx.agent()` returns the **full envelope** `{status,data?,question?,sessionId}`; `blocked` is returned **as-is** (no engine pause in WF3 — that's WF4). (4) Permission presets **read/write/bypass**, default **read** (read = read-only tools + `emit_result`, guaranteed non-mutating). | Findings recommended Arm M to keep the `blocked` terminal value + per-step routing first-class for WF4; ajv because the author declares a JSON Schema and the tool `inputSchema` is JSON Schema too; full-envelope return lets WF4 add the pause without breaking the happy path; the preset set is PRD-fixed (US 26). Spec: `.specs/features/workflows-agent-step/spec.md` (WF3-01..25). | +| AD-013 | 2026-07-29 | **Worktree post-create hook (`worktree-post-create-hook`) scope pinned via 4 owner decisions + a decorator architecture:** (1) The command is declared **repo-locally** in a NEW `\.app\config.json` key `postCreateCommand` (mirrors the existing workspace-level `.app/config.json` reader one level down) — **not** in global settings, so it travels with the repo. (2) A failing hook **keeps the worktree**: `createWorktree` returns `ok:true` plus a `hook` failure payload (exit code + 4000-char output tail); no rollback. (3) The hook runs on **all three create paths** (New Worktree, Start Work, workflow `ctx.worktree.create`). (4) Feedback is **inline in the dialog on failure, silent on success**; the workflow run-timeline detail box is **P2/deferred**. **Architecture:** a `withPostCreateHook(create, deps)` **decorator** (Approach D) wraps `createWorktree` with an identical signature, wired **once** in `index.ts` and assigned to both the IPC handler and `ctxDeps.worktree.create` — so `worktree-manager.ts` (+ its ~40 real-git tests) and `workflow-ctx.ts` are **untouched**, and the run-iff-created rule (`ok && path`) is unit-testable against a fake create with no git and no spawn. The 120 s timeout's process kill stays in the hand-verified `index.ts` spawn seam; only its result *mapping* is unit-tested. | Repo-local won because the init script (`SetupSkills.cmd` in `m:\triade\source\Code`) is already checked in and resolves its own paths from `$PSScriptRoot` — the repo is what knows its init. Keeping the worktree matches the fact that `git worktree add` already succeeded; discarding a valid checkout (plus any base refresh / branch recut) over a fixable script error is the worse failure. All-three-paths because workflow-created worktrees for agents are the case that most needs the skills junctions. The decorator was chosen over a 7th positional param, a trailing options object, and a module-level setter because it is the only option that changes neither the real-git module nor the workflow ctx, and it avoids the parallel-test-hostile global state a setter would introduce. **Accepted trade-off, recorded not buried:** the command is repo content, so cloning an untrusted repo into a registered workspace means its `postCreateCommand` runs on the next create for that repo — no prompt, no allowlist in v1. Spec/design/tasks: `.specs/features/worktree-post-create-hook/` (WPC-01..24; 21 in the P1 slice, WPC-17..19 deferred). | ## Handoff diff --git a/.specs/features/worktree-post-create-hook/design.md b/.specs/features/worktree-post-create-hook/design.md new file mode 100644 index 0000000..b5a2603 --- /dev/null +++ b/.specs/features/worktree-post-create-hook/design.md @@ -0,0 +1,129 @@ +# Worktree Post-Create Hook — Design + +**Spec**: `spec.md` (WPC-01..24) · **Scope**: Medium · **Baseline**: 489 tests / 36 files (main, green) + +--- + +## The one real decision: where the hook attaches + +WPC-10 requires all three create paths to run the hook. They all funnel through +`createWorktree()` (`src/main/worktree-manager.ts:72`), which already takes **6 positional +params** and is covered by ~40 real-git tests. Four ways to get a hook in: + +| Option | Verdict | +| ------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **A** — 7th positional param `hook?: HookRunner` on `createWorktree` | ❌ A 7th positional is unreadable, and it widens `CtxDeps.worktree.create` + the `workflow-ctx` pass-through, so `workflow-ctx.ts` and its tests change for nothing. | +| **B** — module-level `setHookRunner()` setter | ❌ Global mutable state. Vitest runs files in parallel workers; a shared runner is exactly the shared-mutable-state the project's testing conventions avoid. | +| **C** — trailing options object `opts?: { runHook }` | ⚠️ Cleaner than A but still forces the hook concern *into* the real-git module and its 40 tests, and still widens the `CtxDeps` signature. | +| **D** — **decorator wrapper `withPostCreateHook(create, deps)`** ✅ | Same signature in, same signature out → drop-in for both consumers. `worktree-manager.ts` and its 40 real-git tests are **untouched**. `workflow-ctx.ts` is **untouched**. Hook decision logic is unit-testable against a *fake* create — no git, no spawn, fast. | + +**Chosen: D.** One wiring point in `index.ts` serves all three paths: + +``` +index.ts + const createWorktreeWithHook = withPostCreateHook(createWorktree, { + readCommand: repoPostCreateCommand, + runShell: runHookShell // real spawn seam, hand-verified + }) + + handle('worktrees:create', … createWorktreeWithHook(…)) ← New Worktree + Start Work + ctxDeps.worktree.create = createWorktreeWithHook ← workflow ctx.worktree.create +``` + +Because both consumers are assigned the wrapper, **no caller can opt out** — WPC-10 holds +structurally rather than by convention. Per lesson **L-001**, the wrapper and both its +consumers are wired in the *same* task (T4), never split across phases. + +--- + +## Modules + +| Module | New/Mod | Responsibility | Tests | +| -------------------------------------------- | -------- | ----------------------------------------------------------------------------------------------------------------------- | ---------------- | +| `src/shared/worktrees.ts` | modify | `PostCreateHookResult` + optional `hook?` on `CreateWorktreeResult`. Additive — absent key = today's shape (WPC-06). | via T2 | +| `src/main/repo-config.ts` | **new** | `repoPostCreateCommand(repoPath)` → `string \| null`. Reads `\.app\config.json`. Mirrors `workspace-config.ts`. | unit (temp dir) | +| `src/main/post-create-hook.ts` | **new** | `runPostCreateHook()` — env, output tail, exit-code → result mapping. `withPostCreateHook()` — the run-iff-created decorator. | unit (fakes) | +| `src/main/index.ts` | modify | `runHookShell` real spawn (shell + timeout + kill) and the single wiring point above. | none (thin shell) | +| `src/renderer/…/HookFailureNotice.tsx` + css | **new** | Presentational failure panel: created-path note, command, exit code / timeout, output tail. | none (convention) | +| `NewWorktreeDialog.tsx`, `StartWorkDialog.tsx` | modify | Hold `hook` failure state, render the notice, offer the proceed action. | none (convention) | + +`workspace-config.ts`, `worktree-manager.ts`, `workflow-ctx.ts` — **not touched.** + +--- + +## Contracts + +```ts +// src/shared/worktrees.ts +export interface PostCreateHookResult { + ok: boolean // exit code === 0 + command: string // the command as declared + code: number // exit code; -1 for spawn failure or timeout kill + output: string // combined stdout+stderr, last 4000 chars, '' when silent (WPC-23) + timedOut?: boolean // set only on the timeout path (WPC-05) +} +// CreateWorktreeResult gains: hook?: PostCreateHookResult + +// src/main/post-create-hook.ts +export interface HookShellResult { code: number; stdout: string; stderr: string; timedOut?: boolean } +export type HookShell = (cmd: string, opts: { cwd: string; env: NodeJS.ProcessEnv; timeoutMs: number }) => Promise +``` + +**Decision rule (`withPostCreateHook`)** — the hook runs **iff** the inner create reports a +worktree, i.e. `result.ok === true && typeof result.path === 'string'`. Every no-worktree +outcome in WPC-08 fails that test without needing to be enumerated: `conflict:'branch-exists'` +is `ok:false`, and so are the empty-template, target-exists, base-refresh and git-failure +paths. The successful `reuse`/`recreate` paths are `ok:true` with a `path`, so they run the +hook — as WPC-08 requires. + +**Timeout split (honest boundary):** `runPostCreateHook` unit-tests the *mapping* of a +`timedOut` shell result to `hook: { ok:false, code:-1, timedOut:true }` (WPC-05's observable +contract). The actual process kill lives in `runHookShell` via `spawn`'s native `timeout` +option — a thin OS shell, hand-verified per `TESTING.md`, like `runShell` before it. + +--- + +## Sequence + +``` +createWorktreeWithHook(repo, branch, …) + │ + ├─ createWorktree(...) ← unchanged, all existing guards + │ └─ ok:false | conflict ──────────────────→ return as-is, NO hook key (WPC-06/08) + │ + ├─ ok:true + path + │ ├─ repoPostCreateCommand(repoPath) + │ │ └─ null (absent/blank/malformed) ───→ return as-is, NO hook key (WPC-06/07) + │ └─ command + │ └─ runPostCreateHook(cmd, {worktreePath, repoPath, branch}) + │ └─ runHookShell(cmd, {cwd: worktreePath, env: +PLAYGROUND_*, timeoutMs: 120000}) + │ + └─ return { ...result, hook } ← ok stays true even on hook failure (WPC-03) +``` + +--- + +## Renderer flow + +Both dialogs already own `error` / `conflict` / `busy` state and a footer region. Add one +`hookFailure` state: + +``` +result.ok && result.path + ├─ result.hook?.ok === false → setHookFailure({path, hook}) → notice stays open, create button disabled (WPC-16) + └─ otherwise → today's path exactly (close, refresh tree, select) (WPC-15) +``` + +The notice's action calls the same `onCreated(path)` the happy path calls, so dismissing +proceeds with tree refresh + selection rather than unwinding (WPC-14). + +--- + +## Constants + +| Constant | Value | AC | +| ----------------------- | -------- | ------ | +| `HOOK_TIMEOUT_MS` | `120000` | WPC-05 | +| `HOOK_OUTPUT_MAX_CHARS` | `4000` | WPC-11 | +| config file | `\.app\config.json`, key `postCreateCommand` | WPC-01 | +| env vars | `PLAYGROUND_WORKTREE_PATH`, `PLAYGROUND_REPO_PATH`, `PLAYGROUND_BRANCH` | WPC-09 | diff --git a/.specs/features/worktree-post-create-hook/spec.md b/.specs/features/worktree-post-create-hook/spec.md new file mode 100644 index 0000000..08a3cc9 --- /dev/null +++ b/.specs/features/worktree-post-create-hook/spec.md @@ -0,0 +1,261 @@ +# Worktree Post-Create Hook Specification + +## Problem Statement + +A fresh `git worktree add` gives you a checkout, but not a *working* checkout. Repos +increasingly need a one-shot local initialization step after the files land — +`m:\triade\source\Code` ships `SetupSkills.cmd` (→ `SetupSkills.ps1`) whose only job is to +create the `.claude\skills` and `.codex\skills` junctions that let multiple coding agents +share one skills source. Today that script has to be remembered and double-clicked by hand +in every new worktree, and worktrees created by a **workflow** (for an agent to work in) +never get it at all — which is precisely the case that needs it most. + +## Goals + +- [ ] A repo can declare one shell command that runs automatically, with cwd = the new + worktree, after every successful worktree create — from any of the app's three create + paths. +- [ ] A failing init command never costs the user the worktree: the checkout is kept and the + failure is reported with its exit code and output, not swallowed. +- [ ] Zero behavior change for repos that declare nothing. + +## Out of Scope + +Explicitly excluded. Documented to prevent scope creep. + +| Feature | Reason | +| ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Multiple commands / ordered hook list | One command is enough — a repo that needs several chains them in its own `.cmd`/`.ps1`, exactly as `SetupSkills.cmd` already calls `SetupSkills.ps1`. | +| Other lifecycle hooks (pre-create, post-remove, post-checkout) | Only the post-create pain is real today. Adding a hook registry now would design for hooks nobody has asked for. | +| Settings-dialog UI for the command | The command is repo-local and checked in (AD decision below); there is nothing per-machine to edit. A global override can be layered later if needed. | +| Trust prompt / command allowlist | Deliberate: see the security assumption below. The set of repos in a registered workspace is already fully trusted by the app (it runs `git` in them). | +| Process-tree kill on timeout | The timeout kills the spawned shell; a detached grandchild may survive. A real tree-kill (`taskkill /T /F`) is a separate, Windows-specific concern. | +| Streaming live output into a terminal session | Considered and rejected during Specify — needs session lifecycle plumbing and severs the create's knowledge of whether init succeeded. | +| Re-run / retry action for a failed hook | The command is idempotent by convention (the reference script is) and re-runnable by hand from the worktree. An in-app re-run button is a v2 nicety. | +| Reading the hook command from the **new worktree's** copy | The source repo's `.app/config.json` is authoritative — reading the worktree's own copy would let a branch under development change what runs on checkout. | + +--- + +## Assumptions & Open Questions + +Every ambiguity is resolved or recorded here — nothing is left silently unclear. + +| Assumption / decision | Chosen default | Rationale | Confirmed? | +| -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| Where the command is declared | New repo-local `\.app\config.json`, key `postCreateCommand` | Mirrors the existing `\.app\config.json` reader (`workspace-config.ts`) one level down: read-on-use, no caching, malformed → silent fallback. The repo self-describes its own init and it travels with the repo. | **y** | +| Failure semantics | Keep the worktree, return `ok: true` with a `hook` failure payload | `git worktree add` already succeeded — the checkout is valid git. Destroying it (plus any base refresh / branch recut) over a fixable script error is the worse outcome. | **y** | +| Which create paths run it | All three — the hook lives inside `createWorktree()` | One seam, one behavior. Workflow-created worktrees are the case that most needs init (agents need the skills junctions). | **y** | +| Feedback surface | Inline in the dialog on failure; silent on success | No new noise on the happy path; the failure lands in the slot the create errors already use, next to an explicit "the worktree *was* created" note. | **y** | +| **Security: the command comes from repo content** | No confirmation prompt, no allowlist — it just runs | Owner accepted this trade-off when choosing the repo-local home. A registered workspace's repos are already trusted (the app runs `git` in them, and worktree paths are derived from them). Flagged here so it is a recorded decision, not an oversight: **cloning an untrusted repo into a registered workspace means its `postCreateCommand` runs on your next worktree create for that repo.** | **y** | +| Hosting shell | `spawn(cmd, { shell: true, windowsHide: true, cwd: })` — same shape as the existing `runShell` (`index.ts:70`) | `.cmd` files require a shell. Reusing the proven `ctx.sh` runner shape means one spawn idiom in the codebase, and `{code, stdout, stderr}` capture that never throws. | y (agent) | +| Timeout | Fixed **120000 ms**, not configurable | Long enough for a junction/copy/restore script, short enough that a hung hook doesn't wedge the create forever. A knob can be added when a real script needs one. | y (agent) | +| Captured-output bound | Combined stdout+stderr, **last 4000 characters** retained | An unbounded string from a chatty script would be held in the main process and shipped over IPC to the dialog. The tail is what diagnoses a failure. | y (agent) | +| Context handed to the command | Inherited `process.env` plus `PLAYGROUND_WORKTREE_PATH`, `PLAYGROUND_REPO_PATH`, `PLAYGROUND_BRANCH` | The reference script needs none of these (it resolves from `$PSScriptRoot`), but a generic script needs the branch/source-repo without re-deriving them. Cheap and additive. | y (agent) | +| Workflow run-timeline detail box | **P2**, not MVP | It needs a new `StepDetail` variant *and* a `RunDetail` render branch. The hook result is already reachable by a workflow author via the returned `CreateWorktreeResult.hook` without it. | y (agent) | + +**Open questions:** none — all resolved or logged above. + +--- + +## User Stories + +### P1: Repo-declared init command runs after create ⭐ MVP + +**User Story**: As a developer, I want a command my repo declares to run automatically in +each new worktree, so that a fresh worktree is immediately usable (skills junctions in +place) without me remembering to double-click a script. + +**Why P1**: This is the entire feature. Without it there is nothing to report on and nothing +to render. + +**Acceptance Criteria**: + +1. **WPC-01** — WHEN `createWorktree` completes a `git worktree add` successfully AND + `\.app\config.json` contains a non-blank string `postCreateCommand` THEN the + system SHALL execute that command through a shell with `cwd` set to the created worktree + path, before returning. +2. **WPC-02** — WHEN the command exits with code `0` THEN the result SHALL be + `{ ok: true, path: , hook: { ok: true, command: , code: 0, output: } }`. +3. **WPC-03** — WHEN the command exits with a non-zero code `N` THEN the result SHALL be + `{ ok: true, path: , hook: { ok: false, command: , code: N, output: } }` + AND the worktree directory SHALL still exist on disk (no rollback, no `worktree remove`). +4. **WPC-04** — WHEN the command cannot be spawned at all (shell-level spawn error) THEN the + result SHALL carry `hook: { ok: false, code: -1, … }` with the error text included in + `output`, and the worktree SHALL still exist. +5. **WPC-05** — WHEN the command has not exited within **120000 ms** THEN the system SHALL + terminate the spawned shell process and return + `hook: { ok: false, code: -1, timedOut: true, output: }`, and the + worktree SHALL still exist. +6. **WPC-06** — WHEN `\.app\config.json` is absent, unreadable, malformed JSON, or + its `postCreateCommand` is missing / blank / whitespace-only / not a string THEN NO command + SHALL be executed AND the returned result SHALL have **no `hook` property** (identical to + the pre-feature result shape). +7. **WPC-07** — WHEN `\.app\config.json` is malformed JSON THEN the system SHALL log + the ignored file via `console.error` and fall back silently (same stance as + `workspaceTemplates`). +8. **WPC-08** — WHEN a create ends **without** a new worktree — `conflict: 'branch-exists'`, + an empty rendered template name, a pre-existing target path, a blocked base refresh, a + branch live in another worktree, or any failed `git worktree add` — THEN NO command SHALL + be executed. WHEN a create **does** produce a worktree via the `onExisting: 'reuse'` or + `onExisting: 'recreate'` paths THEN the command SHALL run exactly as on the normal path. +9. **WPC-09** — WHEN the command is executed THEN its environment SHALL be the inherited + `process.env` plus `PLAYGROUND_WORKTREE_PATH` (created worktree path), + `PLAYGROUND_REPO_PATH` (source repo path), and `PLAYGROUND_BRANCH` (the branch name). +10. **WPC-10** — WHEN a worktree is created through **any** of the three paths + (`worktrees:create` IPC from New Worktree, the same IPC from Start Work, and workflow + `ctx.worktree.create`) THEN the hook SHALL run, because it is executed inside + `createWorktree` itself and no caller can opt out. +11. **WPC-11** — WHEN the command's captured combined output exceeds 4000 characters THEN + `hook.output` SHALL contain the **last** 4000 characters. + +**Independent Test**: In a temp repo with `.app\config.json` declaring a command that writes +a marker file, call `createWorktree` and assert (a) the marker exists inside the new worktree, +(b) `hook.ok === true`, `hook.code === 0`. Swap in a command that exits 1 and assert +`ok === true`, `hook.ok === false`, `hook.code === 1`, worktree still present. + +--- + +### P1: Hook failure is visible where the user created the worktree ⭐ MVP + +**User Story**: As a developer, I want a failed init command reported in the dialog I just +used, so that I know the worktree exists but is not fully set up — instead of discovering +missing junctions later. + +**Why P1**: Per the failure decision the create returns `ok: true`; without this the failure +is silently discarded by both dialogs and the feature is undetectable in the UI. + +**Acceptance Criteria**: + +1. **WPC-12** — WHEN `worktrees:create` returns `hook.ok === false` from the **New Worktree** + dialog THEN the dialog SHALL remain open and display: a confirmation that the worktree was + created (with its path), the executed command, the exit code (or a timeout label), and the + `hook.output` tail. +2. **WPC-13** — WHEN `worktrees:create` returns `hook.ok === false` from the **Start Work** + dialog THEN the dialog SHALL remain open and display the same four elements as WPC-12. +3. **WPC-14** — WHEN a hook-failure state is shown THEN the dialog SHALL offer an action that + dismisses it and proceeds with the normal post-create flow (tree refresh + select the new + worktree) — the created worktree is never stranded behind an error the user must undo. +4. **WPC-15** — WHEN the result has `hook.ok === true` OR no `hook` property THEN both dialogs + SHALL behave exactly as they do today (close, refresh the tree, select the new worktree) + with no additional UI. +5. **WPC-16** — WHEN a hook-failure state is shown THEN the create button SHALL NOT re-submit + the same create (the worktree already exists; a re-submit would fail on the target-path + guard). + +**Independent Test**: With a repo whose `postCreateCommand` is `exit /b 1`, create a worktree +from each dialog and confirm the failure panel shows command + exit code + output, the +worktree appears in the tree after dismissing, and no error is shown for a repo with no +`.app\config.json`. + +--- + +### P2: Hook result on the workflow run timeline + +**User Story**: As a workflow author, I want a worktree step's init result shown in the run +timeline, so that a failed init in an unattended run is visible without reading the +workflow's own logging. + +**Why P2**: Needs a new shared `StepDetail` variant plus a `RunDetail` render branch. Until +then a workflow author can already read `result.hook` from `ctx.worktree.create` and +`ctx.log`/`ctx.notify` it — so the information is reachable, just not automatic. + +**Acceptance Criteria**: + +1. **WPC-17** — WHEN a `ctx.worktree.create` step's result carries a `hook` THEN the + `step-finished` event SHALL carry a `detail` describing it (command, ok, code, output tail). +2. **WPC-18** — WHEN a `ctx.worktree.create` step's hook failed THEN the step SHALL still be + reported `ok: true` (a hook failure is not a step failure — consistent with WPC-03). +3. **WPC-19** — WHEN a run's `worktree.create` step carries a hook detail THEN `RunDetail` + SHALL render it in the step's detail box, visually marked as failed when `ok` is false. + +--- + +## Edge Cases + +- **WPC-20** — WHEN the created worktree path contains spaces THEN the command SHALL still run + correctly (the path is passed as the spawn `cwd`, never interpolated into the command string). +- **WPC-21** — WHEN `\.app\config.json` contains `postCreateCommand` alongside + `branchTemplate`/`worktreeTemplate` keys THEN each reader SHALL ignore the other's keys — the + hook reader never affects template resolution and vice versa. +- **WPC-22** — WHEN two creates run concurrently (e.g. a workflow run plus a manual create) + THEN each hook SHALL execute independently with its own cwd and captured output; neither + shares state with nor blocks the other. +- **WPC-23** — WHEN the command produces no output at all THEN `hook.output` SHALL be the empty + string (never `undefined`), so consumers need no absent-vs-empty branch. +- **WPC-24** — WHEN the command exits 0 but the worktree is still not properly initialized + (script silently did nothing) THEN the system SHALL report success — the exit code is the only + contract; the app does not verify the script's intent. + +--- + +## Implicit-Requirement Dimensions Sweep + +Medium scope — dimensions obviously present for this domain are covered; the rest are +explicitly N/A. + +| Dimension | Resolution | +| -------------------------------- | ------------------------------------------------------------------------------------------------------- | +| Input validation & bounds | WPC-06 (blank / non-string / malformed), WPC-05 (time bound), WPC-11 (output bound) | +| Failure / partial-failure states | WPC-03, WPC-04, WPC-05 — every failure mode keeps the worktree and reports; WPC-08 defines "no worktree" | +| Observability | WPC-07 (`console.error` on malformed config), WPC-12/13 (surfaced to the user), WPC-17 (run timeline) | +| Concurrency / ordering | WPC-22 (independent), and the hook is strictly ordered after `git worktree add`, before the return | +| State-transition integrity | WPC-08 — hook runs **iff** a worktree was actually created | +| Idempotency / retry | No automatic retry; the hook runs at most once per successful create. Re-running is manual (Out of Scope) | +| Auth boundaries & rate limits | N/A because the command runs in-process on the user's own machine at their own request; no remote caller | +| External-dependency failure | N/A because the hook makes no network or service call — the spawned process is the only dependency, covered by WPC-04/05 | +| Data lifecycle / expiry | N/A because the hook persists nothing — the captured output lives only in the returned result | + +--- + +## Requirement Traceability + +| Requirement ID | Story | Phase | Status | +| -------------- | ------------------------------ | ----- | ------- | +| WPC-01 | P1: Init command runs | Tasks | Pending | +| WPC-02 | P1: Init command runs | Tasks | Pending | +| WPC-03 | P1: Init command runs | Tasks | Pending | +| WPC-04 | P1: Init command runs | Tasks | Pending | +| WPC-05 | P1: Init command runs | Tasks | Pending | +| WPC-06 | P1: Init command runs | Tasks | Pending | +| WPC-07 | P1: Init command runs | Tasks | Pending | +| WPC-08 | P1: Init command runs | Tasks | Pending | +| WPC-09 | P1: Init command runs | Tasks | Pending | +| WPC-10 | P1: Init command runs | Tasks | Pending | +| WPC-11 | P1: Init command runs | Tasks | Pending | +| WPC-12 | P1: Hook failure is visible | Tasks | Pending | +| WPC-13 | P1: Hook failure is visible | Tasks | Pending | +| WPC-14 | P1: Hook failure is visible | Tasks | Pending | +| WPC-15 | P1: Hook failure is visible | Tasks | Pending | +| WPC-16 | P1: Hook failure is visible | Tasks | Pending | +| WPC-17 | P2: Run-timeline detail | - | Pending | +| WPC-18 | P2: Run-timeline detail | - | Pending | +| WPC-19 | P2: Run-timeline detail | - | Pending | +| WPC-20 | Edge case | Tasks | Pending | +| WPC-21 | Edge case | Tasks | Pending | +| WPC-22 | Edge case | Tasks | Pending | +| WPC-23 | Edge case | Tasks | Pending | +| WPC-24 | Edge case | Tasks | Pending | + +**ID format:** `WPC-[NUMBER]` + +**Coverage:** 24 total — 21 in the P1 MVP slice (WPC-01..16, WPC-20..24), 3 deferred to P2 +(WPC-17..19). + +**Unit-testable vs hand-verified** (per `.specs/codebase/TESTING.md`): WPC-01..11 and +WPC-20..24 are main-process logic → **unit tests** (real-temp-dir + injected-fake runner, no +real spawn in the assertions of the decision logic). WPC-12..16 and WPC-19 are renderer → +**hand-verified** by convention. The actual `spawn(shell:true)` seam in `index.ts` is a thin +OS shell → hand-verified. + +--- + +## Success Criteria + +- [ ] Creating a worktree for `m:\triade\source\Code` (with `.app\config.json` declaring + `SetupSkills.cmd`) leaves `.claude\skills` and `.codex\skills` junctions present in the + new worktree, with no manual step. +- [ ] A worktree created by a **workflow** for that repo gets the same junctions. +- [ ] A repo with no `.app\config.json` behaves byte-identically to today (existing + `worktree-manager` tests pass unchanged). +- [ ] A deliberately-failing command yields a worktree that exists, plus a dialog showing the + command, its exit code, and its output. diff --git a/.specs/features/worktree-post-create-hook/tasks.md b/.specs/features/worktree-post-create-hook/tasks.md new file mode 100644 index 0000000..835b377 --- /dev/null +++ b/.specs/features/worktree-post-create-hook/tasks.md @@ -0,0 +1,350 @@ +# Worktree Post-Create Hook — Tasks + +## Execution Protocol (MANDATORY -- do not skip) + +Implement these tasks with the `tlc-spec-driven` skill: **activate it by name and follow its +Execute flow and Critical Rules.** Do not search for skill files by filesystem path. The skill +is the source of truth for the full flow (per-task cycle, sub-agent delegation, adequacy +review, Verifier, discrimination sensor). + +**If the skill cannot be activated, STOP and tell the user — do not proceed without it.** + +--- + +**Design**: `.specs/features/worktree-post-create-hook/design.md` +**Status**: Approved +**Branch**: `feature/worktree-post-create-hook` +**Baseline**: 489 tests / 36 files (green on `main`, verified) + +--- + +## Test Coverage Matrix + +> Generated from codebase, project guidelines, and spec. Guidelines found: +> `.specs/codebase/TESTING.md` (authoritative — coverage matrix + gate table), +> `.specs/codebase/CONVENTIONS.md`, `vitest.config.ts` +> (`include: ['src/**/*.test.ts', 'scripts/**/*.test.ts']`). + +| Code Layer | Required Test Type | Coverage Expectation | Location Pattern | Run Command | +| --------------------------------------------------------------------- | ------------------ | -------------------------------------------------------------------- | ----------------------------- | ----------- | +| Main-process deep modules with logic (`repo-config`, `post-create-hook`) | **unit** | All branches; 1:1 to spec ACs; every listed edge case has a test | `src/main/.test.ts` | `npm test` | +| Shared types (`src/shared/worktrees.ts`) | none | — (typecheck gate only; additive optional field) | — | build gate | +| Thin OS/Electron shells (`index.ts` spawn seam + IPC/ctxDeps wiring) | none | — (hand-verified per TESTING.md; only extracted logic is unit-tested) | `src/main/index.ts` | build gate | +| Renderer React components (`HookFailureNotice`, both dialogs) | none | — (CDP smoke + visual pass, by convention) | `src/renderer/**` | build gate | + +**Provenance note:** `TESTING.md` explicitly excludes renderer components and thin +OS/Electron shells from unit tests ("extract pure/decision logic into a testable seam, +unit-test that seam, and hand-verify the thin OS/Electron shell around it"). This feature +follows that split exactly — which is *why* the design chose the decorator wrapper: it moves +the hook's decision logic out of the un-unit-testable spawn shell and into a seam driven by +hand-rolled fakes. No mocking library (`vi.mock` is used nowhere in this repo). + +## Parallelism Assessment + +> Generated from codebase. + +| Test Type | Parallel-Safe? | Isolation Model | Evidence | +| -------------------- | -------------- | ---------------------------------------------------------------------- | ------------------------------------------------- | +| Unit (real-temp-dir) | **Yes** | Per-test `mkdtempSync(join(tmpdir(),'wtm-…'))` + `rmSync` teardown | `workspace-config.test.ts`, `config-store.test.ts` | +| Unit (injected fake) | **Yes** | Hand-rolled fakes constructed per test; no globals, no `vi.mock` | `task-board.test.ts` (`stubSource`) | +| Renderer / shells | n/a | No unit tests by convention | `TESTING.md` "deliberately NOT unit-tested" | + +## Gate Check Commands + +> Generated from codebase (`package.json` scripts + `TESTING.md` gate table). + +| Gate Level | When to Use | Command | +| ---------- | ------------------------------------------------- | ---------------------------------------------- | +| Quick | After tasks with unit tests only | `npm test` | +| Full | After a logic-bearing task / before PR | `npm run typecheck && npm run lint && npm test` | +| Build | After phase completion or no-test (wiring/UI) tasks | `npm run build` (= `typecheck` + `electron-vite build`) | + +**Build-gate note:** `TESTING.md` lists `npm run build:win` for the Build level. That +heavier gate exists for the packaged-asset pitfalls recorded in the esbuild lesson +(asar paths, unpacked binaries). This feature adds **no packaged asset and no new +dependency** — it spawns a shell already available at runtime — so `npm run build` is the +appropriate build gate. A single `build:win` is still run once at the end of Phase 3 to +confirm the renderer additions package cleanly. + +--- + +## Execution Plan + +**3 phases → executed inline** (the sub-agent offer threshold is >3 phases). The always-on +Verifier still runs as a fresh sub-agent after T6. + +### Phase 1: Main-process logic (Sequential) + +``` +T1 → T2 → T3 +``` + +### Phase 2: Wiring (Sequential) + +``` +T3 → T4 +``` + +### Phase 3: Renderer (Sequential) + +``` +T4 → T5 → T6 +``` + +--- + +## Task Breakdown + +### T1: Repo-local config reader + +**What**: `repoPostCreateCommand(repoPath)` returning the trimmed `postCreateCommand` string +from `\.app\config.json`, or `null`. +**Where**: `src/main/repo-config.ts` (new), `src/main/repo-config.test.ts` (new) +**Depends on**: None +**Reuses**: `src/main/workspace-config.ts` — same read-on-use / `stringOrNull` / +`console.error`-on-malformed shape; `src/main/workspace-config.test.ts` — same temp-dir test +harness. +**Requirement**: WPC-06 (config half), WPC-07, WPC-21 + +**Tools**: MCP: NONE · Skill: NONE + +**Done when**: + +- [ ] A valid file with `postCreateCommand: "SetupSkills.cmd"` returns `'SetupSkills.cmd'` +- [ ] Surrounding whitespace is trimmed (`" SetupSkills.cmd "` → `'SetupSkills.cmd'`) +- [ ] Missing file, missing key, blank/whitespace-only value, and non-string values (number, + object) each return `null` (WPC-06) +- [ ] Malformed JSON returns `null` **and** logs via `console.error` (WPC-07) +- [ ] A file carrying `branchTemplate`/`worktreeTemplate` alongside `postCreateCommand` + returns the command and ignores the template keys (WPC-21) +- [ ] Quick gate passes: `npm test` +- [ ] Test count: 489 → **498** (+9), zero deletions + +**Tests**: unit · **Gate**: quick +**Commit**: `feat(worktree): read repo-local postCreateCommand from .app/config.json` + +--- + +### T2: Hook runner — result mapping + +**What**: `PostCreateHookResult` in shared + `runPostCreateHook()` mapping a `HookShell` +outcome to that result (env injection, output tail, code/timeout classification). +**Where**: `src/shared/worktrees.ts` (modify), `src/main/post-create-hook.ts` (new), +`src/main/post-create-hook.test.ts` (new) +**Depends on**: T1 (none in code; ordered for a clean commit sequence) +**Reuses**: `ShellResult` shape from `workflow-ctx.ts`; `runShell` capture semantics +(`index.ts:70`) as the port contract. +**Requirement**: WPC-02, WPC-03 (payload half), WPC-04, WPC-05 (mapping half), WPC-09, +WPC-11, WPC-20, WPC-23, WPC-24 + +**Tools**: MCP: NONE · Skill: NONE + +**Done when**: + +- [ ] Type is co-located in `src/shared/worktrees.ts` with `hook?: PostCreateHookResult` added + to `CreateWorktreeResult`; `npm run typecheck` clean +- [ ] Exit 0 → `{ok:true, code:0, command, output}` (WPC-02) +- [ ] Exit N≠0 → `{ok:false, code:N, command, output}` (WPC-03 payload) +- [ ] Shell reports `code:-1` spawn error → `{ok:false, code:-1}` with the error text present + in `output` (WPC-04) +- [ ] Shell reports `timedOut:true` → `{ok:false, code:-1, timedOut:true}` (WPC-05 mapping) +- [ ] The shell receives `cwd` = worktree path and an env containing + `PLAYGROUND_WORKTREE_PATH`, `PLAYGROUND_REPO_PATH`, `PLAYGROUND_BRANCH` **with the + correct values**, plus inherited `process.env` entries (WPC-09) +- [ ] The shell receives `timeoutMs: 120000` (WPC-05 constant) +- [ ] Combined output longer than 4000 chars keeps the **last** 4000 (WPC-11); stdout and + stderr are both represented +- [ ] No output at all → `output === ''`, never `undefined` (WPC-23) +- [ ] A worktree path containing spaces reaches the shell as `cwd` unmodified and is never + interpolated into the command string (WPC-20) +- [ ] Exit 0 with a no-op command still reports `ok:true` (WPC-24) +- [ ] Quick gate passes: `npm test` +- [ ] Test count: 498 → **510** (+12), zero deletions + +**Tests**: unit · **Gate**: quick +**Commit**: `feat(worktree): add post-create hook runner with env, output tail and timeout mapping` + +--- + +### T3: Run-iff-created decorator + +**What**: `withPostCreateHook(create, deps)` — wraps a `createWorktree`-shaped function, +running the hook only when a worktree was actually produced and attaching `hook` to the result. +**Where**: `src/main/post-create-hook.ts` (modify), `src/main/post-create-hook.test.ts` (modify) +**Depends on**: T1, T2 +**Reuses**: T1's reader + T2's runner, both injected as `deps` (project DI-with-hand-rolled-fakes +convention, `task-board.test.ts`). +**Requirement**: WPC-01, WPC-03 (no-rollback half), WPC-06 (no-hook-key half), WPC-08, WPC-22 + +**Tools**: MCP: NONE · Skill: NONE + +**Done when**: + +- [ ] Signature is identical to `createWorktree`'s (drop-in for both consumers); typecheck clean +- [ ] `ok:true` + `path` + a configured command → hook runs with `cwd` = the returned path, and + the result is `{...inner, hook}` (WPC-01) +- [ ] Hook failure leaves `ok:true` and `path` intact, and the wrapper issues **no** remove call + (WPC-03 — asserted via a fake that would record one) +- [ ] `conflict:'branch-exists'` → runner never invoked, result has **no** `hook` property + (`'hook' in result === false`) (WPC-08) +- [ ] `ok:false` with an `error` (empty template / target exists / refresh blocked / git failure) + → runner never invoked, no `hook` property (WPC-08) +- [ ] `ok:true` reached through `onExisting:'reuse'` and through `onExisting:'recreate'` → hook + **does** run (WPC-08 second half) +- [ ] No configured command → runner never invoked, no `hook` property (WPC-06) +- [ ] Two concurrent wrapped creates each run their hook with their own cwd; neither's output + leaks into the other (WPC-22) +- [ ] Quick gate passes: `npm test` +- [ ] Test count: 510 → **520** (+10), zero deletions + +**Tests**: unit · **Gate**: quick +**Commit**: `feat(worktree): run the post-create hook only when a worktree was created` + +--- + +### T4: Wire the real spawn seam and both consumers + +**What**: `runHookShell` (real `spawn` with `shell:true` + native `timeout` + kill detection) +and the single `withPostCreateHook(createWorktree, …)` wiring assigned to **both** the +`worktrees:create` IPC handler and `ctxDeps.worktree.create`. +**Where**: `src/main/index.ts` (modify) +**Depends on**: T3 +**Reuses**: `runShell` (`index.ts:70`) as the spawn/capture template. +**Requirement**: WPC-10, WPC-05 (kill half) + +**Tools**: MCP: NONE · Skill: NONE + +**Done when**: + +- [ ] `runHookShell` spawns through a shell (`.cmd` support), captures combined stdout/stderr, + never throws, and maps a `timeout`-triggered kill to `timedOut:true` +- [ ] **One** wrapper instance is assigned to both the IPC handler and `ctxDeps.worktree.create` + — no caller can opt out (WPC-10); `worktree-manager.ts` and `workflow-ctx.ts` remain + unmodified (verified by `git diff --stat`) +- [ ] Build gate passes: `npm run build` +- [ ] Full gate passes: `npm run typecheck && npm run lint && npm test` — test count still + **520**, zero deletions + +**Tests**: none (thin OS/Electron shell — hand-verified per TESTING.md) · **Gate**: build +**Commit**: `feat(worktree): wire the post-create hook into both create paths` + +--- + +### T5: Hook failure notice component + +**What**: One presentational `HookFailureNotice` rendering the created path, the command, the +exit code (or timeout label), and the output tail, plus its stylesheet. +**Where**: `src/renderer/src/components/HookFailureNotice.tsx` (new), `HookFailureNotice.css` (new) +**Depends on**: T4 +**Reuses**: `BranchExistsChoice.tsx` — the established in-dialog footer-region resolver pattern +(props-only, `busy` flag, no data fetching); existing CSS tokens. +**Requirement**: WPC-12/WPC-13 (presentation half), WPC-14 (action) + +**Tools**: MCP: NONE · Skill: NONE + +**Done when**: + +- [ ] Renders all four required elements: created-worktree path, executed command, exit code + (or an explicit timeout label when `timedOut`), and the `output` tail +- [ ] Empty `output` renders without an empty bordered block +- [ ] Exposes a single proceed action via props; no IPC calls of its own +- [ ] Build gate passes: `npm run build` + +**Tests**: none (renderer — CDP smoke + visual, by convention) · **Gate**: build +**Commit**: `feat(worktree): add hook failure notice component` + +--- + +### T6: Surface hook failure in both dialogs + +**What**: Both create dialogs hold a `hookFailure` state, render `HookFailureNotice` on +`hook.ok === false`, and keep today's behavior otherwise. +**Where**: `src/renderer/src/components/NewWorktreeDialog.tsx` (modify), +`src/renderer/src/components/StartWorkDialog.tsx` (modify) +**Depends on**: T4, T5 +**Reuses**: each dialog's existing `error`/`conflict`/`busy` state machine and footer region. +**Requirement**: WPC-12, WPC-13, WPC-14, WPC-15, WPC-16 + +**Tools**: MCP: NONE · Skill: NONE + +**Done when**: + +- [ ] `hook.ok === false` → dialog stays open showing the notice (WPC-12 New Worktree, + WPC-13 Start Work) +- [ ] The notice's action runs the same post-create flow as the happy path (tree refresh + + select the new worktree) (WPC-14) +- [ ] `hook.ok === true` or absent `hook` → behavior byte-identical to today (WPC-15) +- [ ] While the notice is shown the create button cannot re-submit (WPC-16) +- [ ] Full gate passes: `npm run typecheck && npm run lint && npm test` — test count still + **520**, zero deletions +- [ ] `npm run build:win` completes (end-of-phase packaging confirmation) + +**Tests**: none (renderer — CDP smoke + visual, by convention) · **Gate**: build +**Commit**: `feat(worktree): surface post-create hook failures in the create dialogs` + +--- + +## Task Granularity Check + +| Task | Scope | Status | +| ------------------------------- | -------------------------------------------- | ----------- | +| T1: repo config reader | 1 function + its tests | ✅ Granular | +| T2: hook runner mapping | 1 function + 1 shared type | ✅ Granular | +| T3: run-iff-created decorator | 1 function (same cohesive module as T2) | ✅ Granular | +| T4: spawn seam + wiring | 1 seam + 1 wiring point, one file | ✅ Granular | +| T5: notice component | 1 component + its CSS | ✅ Granular | +| T6: dialog integration | 2 files, one identical cohesive change each | ⚠️ OK — cohesive; splitting would leave the shared notice half-wired | + +## Diagram-Definition Cross-Check + +| Task | Depends On (task body) | Diagram Shows | Status | +| ---- | ---------------------- | ------------- | -------- | +| T1 | None | (phase start) | ✅ Match | +| T2 | T1 | T1 → T2 | ✅ Match | +| T3 | T1, T2 | T2 → T3 (T1 transitively via T2) | ✅ Match | +| T4 | T3 | T3 → T4 | ✅ Match | +| T5 | T4 | T4 → T5 | ✅ Match | +| T6 | T4, T5 | T5 → T6 (T4 transitively via T5) | ✅ Match | + +No task carries `[P]` — every phase is sequential, so no parallel-safety conflict is possible. + +## Test Co-location Validation + +| Task | Code Layer Created/Modified | Matrix Requires | Task Says | Status | +| ---- | ------------------------------------------------ | --------------- | --------- | ------ | +| T1 | Main-process deep module (`repo-config`) | unit | unit | ✅ OK | +| T2 | Main-process deep module (`post-create-hook`) + shared type | unit (highest) | unit | ✅ OK | +| T3 | Main-process deep module (`post-create-hook`) | unit | unit | ✅ OK | +| T4 | Thin OS/Electron shell (`index.ts` spawn + wiring) | none | none | ✅ OK | +| T5 | Renderer component | none | none | ✅ OK | +| T6 | Renderer components | none | none | ✅ OK | + +**No deferral:** every task that creates unit-testable logic writes its own tests in the same +commit. T4–T6 are `Tests: none` because the matrix says `none` for those layers (thin shell / +renderer), not because their tests were pushed elsewhere. + +--- + +## Requirement → Task Map + +| Requirement | Task | +| ---------------------- | -------- | +| WPC-01 | T3 | +| WPC-02 | T2 | +| WPC-03 | T2 + T3 | +| WPC-04 | T2 | +| WPC-05 | T2 (mapping) + T4 (kill) | +| WPC-06 | T1 + T3 | +| WPC-07 | T1 | +| WPC-08 | T3 | +| WPC-09 | T2 | +| WPC-10 | T4 | +| WPC-11 | T2 | +| WPC-12, WPC-13 | T5 + T6 | +| WPC-14, WPC-15, WPC-16 | T6 | +| WPC-17..19 (P2) | — deferred, out of this slice | +| WPC-20, WPC-23, WPC-24 | T2 | +| WPC-21 | T1 | +| WPC-22 | T3 | + +**Coverage:** 21 of 21 P1 requirements mapped to a task. 0 unmapped. From 7732a89148d6c7d089f93f03ba71a7264bb3ca97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 17:36:04 -0300 Subject: [PATCH 02/13] feat(worktree): read repo-local postCreateCommand from .app/config.json Mirrors workspaceTemplates one level down: read-on-use, trimmed, null for an absent/blank/non-string value, and null plus a console.error for malformed JSON. Repo and workspace keys stay mutually independent. Covers WPC-06 (config half), WPC-07, WPC-21. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/repo-config.test.ts | 98 ++++++++++++++++++++++++++++++++++++ src/main/repo-config.ts | 39 ++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 src/main/repo-config.test.ts create mode 100644 src/main/repo-config.ts diff --git a/src/main/repo-config.test.ts b/src/main/repo-config.test.ts new file mode 100644 index 0000000..909267b --- /dev/null +++ b/src/main/repo-config.test.ts @@ -0,0 +1,98 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { repoPostCreateCommand } from './repo-config' +import { workspaceTemplates } from './workspace-config' + +describe('repoPostCreateCommand', () => { + let dir: string + let logged: unknown[][] + let realConsoleError: typeof console.error + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'wtm-repoconfig-')) + // Hand-rolled capture rather than a mocking library (no vi.mock in this repo). + logged = [] + realConsoleError = console.error + console.error = (...args: unknown[]): void => { + logged.push(args) + } + }) + + afterEach(() => { + console.error = realConsoleError + rmSync(dir, { recursive: true, force: true }) + }) + + const writeConfig = (content: string): void => { + mkdirSync(join(dir, '.app'), { recursive: true }) + writeFileSync(join(dir, '.app', 'config.json'), content, 'utf8') + } + + it('returns the postCreateCommand from .app/config.json', () => { + writeConfig('{ "postCreateCommand": "SetupSkills.cmd" }') + + expect(repoPostCreateCommand(dir)).toBe('SetupSkills.cmd') + }) + + it('trims surrounding whitespace from the command', () => { + writeConfig('{ "postCreateCommand": " SetupSkills.cmd " }') + + expect(repoPostCreateCommand(dir)).toBe('SetupSkills.cmd') + }) + + it('returns null when the repo has no .app directory', () => { + expect(repoPostCreateCommand(dir)).toBeNull() + }) + + it('returns null when .app exists but config.json does not', () => { + mkdirSync(join(dir, '.app')) + + expect(repoPostCreateCommand(dir)).toBeNull() + }) + + it('returns null when postCreateCommand is absent', () => { + writeConfig('{ "somethingElse": true }') + + expect(repoPostCreateCommand(dir)).toBeNull() + }) + + it('returns null for a blank command', () => { + writeConfig('{ "postCreateCommand": " " }') + + expect(repoPostCreateCommand(dir)).toBeNull() + }) + + it('returns null for a non-string command', () => { + writeConfig('{ "postCreateCommand": 42 }') + expect(repoPostCreateCommand(dir)).toBeNull() + + writeConfig('{ "postCreateCommand": { "cmd": "SetupSkills.cmd" } }') + expect(repoPostCreateCommand(dir)).toBeNull() + }) + + it('returns null and logs the ignored file on malformed JSON', () => { + writeConfig('{ this is not json') + + expect(repoPostCreateCommand(dir)).toBeNull() + expect(logged).toHaveLength(1) + expect(String(logged[0][0])).toContain(join(dir, '.app', 'config.json')) + }) + + it('keeps the hook command and the template keys independent', () => { + writeConfig( + '{ "postCreateCommand": "SetupSkills.cmd", "branchTemplate": "task/{id}", "worktreeTemplate": "{id}" }' + ) + + expect(repoPostCreateCommand(dir)).toBe('SetupSkills.cmd') + expect(workspaceTemplates(dir)).toEqual({ + branchTemplate: 'task/{id}', + worktreeTemplate: '{id}' + }) + }) + + it('returns null for a repo path that does not exist', () => { + expect(repoPostCreateCommand(join(dir, 'nope'))).toBeNull() + }) +}) diff --git a/src/main/repo-config.ts b/src/main/repo-config.ts new file mode 100644 index 0000000..7f02057 --- /dev/null +++ b/src/main/repo-config.ts @@ -0,0 +1,39 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' + +/** + * Per-repo config (WPC-01): a hand-authored, optionally checked-in + * `/.app/config.json` carrying the command a freshly created worktree runs + * to initialize itself (e.g. a script that replicates agent skills into the new + * checkout). Read on use — no caching, no watching — so an on-disk edit takes + * effect at the next create. + * + * Deliberately separate from `workspaceTemplates`: same file name, different + * level (repo vs workspace), independent keys — neither reader sees the other's + * (WPC-21). + * + * Returns the trimmed command, or null when the file or key is absent, blank, + * not a string, or unreadable — in which case no hook runs at all (WPC-06). + * Malformed JSON also yields null, but is logged via console.error (WPC-07). + */ +export function repoPostCreateCommand(repoPath: string): string | null { + const filePath = join(repoPath, '.app', 'config.json') + let raw: string + try { + raw = readFileSync(filePath, 'utf8') + } catch { + return null + } + try { + const parsed: unknown = JSON.parse(raw) + return stringOrNull((parsed as Record)?.postCreateCommand) + } catch (err) { + console.error(`Ignoring malformed repo config ${filePath}:`, err) + return null + } +} + +/** Trimmed non-empty string, else null (numbers/objects/blank all collapse to null). */ +function stringOrNull(value: unknown): string | null { + return typeof value === 'string' && value.trim() !== '' ? value.trim() : null +} From 4859446789ed20a2ba17f4632aeaeec7333fcb6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 17:47:18 -0300 Subject: [PATCH 03/13] feat(worktree): add post-create hook runner with env, output tail and timeout mapping runPostCreateHook drives an injected HookShell with cwd set to the new worktree and the three PLAYGROUND_* context vars layered over process.env, then shapes the outcome: exit 0 is success, a non-zero exit, spawn failure (-1) and timeout kill (-1 + timedOut) all report ok:false with the evidence. Combined stdout+stderr is bounded to the last 4000 chars and is '' when the command is silent. Covers WPC-02, WPC-03 (payload), WPC-04, WPC-05 (mapping), WPC-09, WPC-11, WPC-20, WPC-23, WPC-24. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/post-create-hook.test.ts | 145 ++++++++++++++++++++++++++++++ src/main/post-create-hook.ts | 81 +++++++++++++++++ src/shared/worktrees.ts | 27 ++++++ 3 files changed, 253 insertions(+) create mode 100644 src/main/post-create-hook.test.ts create mode 100644 src/main/post-create-hook.ts diff --git a/src/main/post-create-hook.test.ts b/src/main/post-create-hook.test.ts new file mode 100644 index 0000000..dd41888 --- /dev/null +++ b/src/main/post-create-hook.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, it } from 'vitest' +import type { HookShell, HookShellResult } from './post-create-hook' +import { HOOK_OUTPUT_MAX_CHARS, HOOK_TIMEOUT_MS, runPostCreateHook } from './post-create-hook' + +interface ShellCall { + cmd: string + cwd: string + env: NodeJS.ProcessEnv + timeoutMs: number +} + +/** Hand-rolled HookShell fake (no mocking library in this repo) that records its call. */ +function fakeShell(result: Partial): { shell: HookShell; calls: ShellCall[] } { + const calls: ShellCall[] = [] + const shell: HookShell = (cmd, opts) => { + calls.push({ cmd, cwd: opts.cwd, env: opts.env, timeoutMs: opts.timeoutMs }) + return Promise.resolve({ code: 0, stdout: '', stderr: '', ...result }) + } + return { shell, calls } +} + +const ctx = { + worktreePath: 'M:\\triade\\source\\Code-feature-x', + repoPath: 'M:\\triade\\source\\Code', + branch: 'feature/x' +} + +describe('runPostCreateHook', () => { + it('reports success when the command exits 0', async () => { + const { shell } = fakeShell({ code: 0, stdout: 'junctions created' }) + + const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(hook.ok).toBe(true) + expect(hook.code).toBe(0) + expect(hook.command).toBe('SetupSkills.cmd') + expect(hook.output).toBe('junctions created') + expect(hook.timedOut).toBeUndefined() + }) + + it('reports failure with the exit code when the command exits non-zero', async () => { + const { shell } = fakeShell({ code: 1, stderr: 'ERRO: a pasta de origem nao existe' }) + + const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(hook.ok).toBe(false) + expect(hook.code).toBe(1) + expect(hook.output).toBe('ERRO: a pasta de origem nao existe') + }) + + it('reports a spawn failure as code -1 carrying the error text', async () => { + const { shell } = fakeShell({ code: -1, stderr: 'Error: spawn EACCES' }) + + const hook = await runPostCreateHook('nope.cmd', ctx, shell) + + expect(hook.ok).toBe(false) + expect(hook.code).toBe(-1) + expect(hook.output).toContain('spawn EACCES') + }) + + it('flags a timeout kill as code -1 with timedOut set', async () => { + const { shell } = fakeShell({ code: -1, timedOut: true, stdout: 'Repo raiz : ...' }) + + const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(hook.ok).toBe(false) + expect(hook.code).toBe(-1) + expect(hook.timedOut).toBe(true) + expect(hook.output).toBe('Repo raiz : ...') + }) + + it('passes the worktree context to the command as PLAYGROUND_* env vars', async () => { + const { shell, calls } = fakeShell({ code: 0 }) + + await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(calls[0].env.PLAYGROUND_WORKTREE_PATH).toBe('M:\\triade\\source\\Code-feature-x') + expect(calls[0].env.PLAYGROUND_REPO_PATH).toBe('M:\\triade\\source\\Code') + expect(calls[0].env.PLAYGROUND_BRANCH).toBe('feature/x') + }) + + it('layers the context vars over the inherited environment', async () => { + const { shell, calls } = fakeShell({ code: 0 }) + + await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(calls[0].env.PATH).toBe(process.env.PATH) + }) + + it('gives the command the 120s timeout budget', async () => { + const { shell, calls } = fakeShell({ code: 0 }) + + await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(calls[0].timeoutMs).toBe(120000) + expect(HOOK_TIMEOUT_MS).toBe(120000) + }) + + it('runs in the worktree via cwd, leaving a spaced path out of the command string', async () => { + const spaced = { + worktreePath: 'M:\\my repos\\Code feature x', + repoPath: 'M:\\my repos\\Code', + branch: 'feature/x' + } + const { shell, calls } = fakeShell({ code: 0 }) + + await runPostCreateHook('SetupSkills.cmd', spaced, shell) + + expect(calls[0].cwd).toBe('M:\\my repos\\Code feature x') + expect(calls[0].cmd).toBe('SetupSkills.cmd') + }) + + it('keeps only the last 4000 characters of a chatty command', async () => { + const { shell } = fakeShell({ code: 0, stdout: 'x'.repeat(4100) + 'TAIL' }) + + const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(hook.output).toHaveLength(HOOK_OUTPUT_MAX_CHARS) + expect(hook.output.endsWith('TAIL')).toBe(true) + }) + + it('represents both stdout and stderr in the captured output', async () => { + const { shell } = fakeShell({ code: 1, stdout: 'started', stderr: 'then failed' }) + + const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(hook.output).toBe('started\nthen failed') + }) + + it('reports an empty string when the command is silent', async () => { + const { shell } = fakeShell({ code: 0, stdout: '', stderr: '' }) + + const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) + + expect(hook.output).toBe('') + }) + + it('reports success for a command that exits 0 without doing anything', async () => { + const { shell } = fakeShell({ code: 0, stdout: '', stderr: '' }) + + const hook = await runPostCreateHook('rem no-op', ctx, shell) + + expect(hook.ok).toBe(true) + }) +}) diff --git a/src/main/post-create-hook.ts b/src/main/post-create-hook.ts new file mode 100644 index 0000000..4d774e5 --- /dev/null +++ b/src/main/post-create-hook.ts @@ -0,0 +1,81 @@ +import type { PostCreateHookResult } from '../shared/worktrees' + +/** How long a repo's init command may run before it is killed (WPC-05). */ +export const HOOK_TIMEOUT_MS = 120000 + +/** Upper bound on the captured output carried back to the caller (WPC-11). */ +export const HOOK_OUTPUT_MAX_CHARS = 4000 + +/** What a hook shell reports back; `timedOut` marks the killed-for-time path. */ +export interface HookShellResult { + code: number + stdout: string + stderr: string + timedOut?: boolean +} + +/** + * The one I/O seam of the hook: runs `cmd` through a shell and captures its + * outcome. Never throws — a spawn failure comes back as `code: -1`. The real + * implementation lives in `index.ts` (thin, hand-verified, like `runShell`); + * tests inject a fake. + */ +export type HookShell = ( + cmd: string, + opts: { cwd: string; env: NodeJS.ProcessEnv; timeoutMs: number } +) => Promise + +/** Where the hook runs and what it is initializing. */ +export interface HookContext { + worktreePath: string + repoPath: string + branch: string +} + +/** + * Runs a repo's post-create command in a freshly created worktree and shapes the + * outcome (WPC-01). The command runs with `cwd` set to the worktree — never + * interpolated into the command string, so a path with spaces needs no quoting + * (WPC-20) — and with the three `PLAYGROUND_*` context variables layered over the + * inherited environment (WPC-09). + * + * Success is exactly "exited 0" (WPC-02/WPC-24): a command that exits 0 without + * doing anything is reported as a success, because the exit code is the whole + * contract. Everything else — a non-zero exit (WPC-03), a spawn failure (WPC-04), + * a timeout kill (WPC-05) — comes back `ok: false` with the evidence attached. + */ +export async function runPostCreateHook( + command: string, + ctx: HookContext, + shell: HookShell +): Promise { + const result = await shell(command, { + cwd: ctx.worktreePath, + env: { + ...process.env, + PLAYGROUND_WORKTREE_PATH: ctx.worktreePath, + PLAYGROUND_REPO_PATH: ctx.repoPath, + PLAYGROUND_BRANCH: ctx.branch + }, + timeoutMs: HOOK_TIMEOUT_MS + }) + return { + ok: result.code === 0, + command, + code: result.code, + output: combinedTail(result), + ...(result.timedOut === true ? { timedOut: true } : {}) + } +} + +/** + * Both streams in one field, newest-last, bounded to the last + * `HOOK_OUTPUT_MAX_CHARS` characters — the tail is what diagnoses a failure, and + * an unbounded string would be held in main and shipped over IPC. A silent + * command yields '' rather than undefined, so consumers need no absent-vs-empty + * branch (WPC-23). + */ +function combinedTail({ stdout, stderr }: HookShellResult): string { + const combined = [stdout, stderr].filter((stream) => stream !== '').join('\n') + return combined.length > HOOK_OUTPUT_MAX_CHARS ? combined.slice(-HOOK_OUTPUT_MAX_CHARS) : combined +} diff --git a/src/shared/worktrees.ts b/src/shared/worktrees.ts index e67bad3..55d1abb 100644 --- a/src/shared/worktrees.ts +++ b/src/shared/worktrees.ts @@ -49,6 +49,27 @@ export function worktreePathFor(repoPath: string, branch: string, template?: str return `${parent}${sep}${worktreeNameFor(repoPath, branch, template)}` } +/** + * Outcome of the repo-declared post-create command (WPC-02..05). Present on a + * create result only when a command actually ran: absent means the repo declared + * none, so a consumer distinguishes "no hook" from "hook succeeded" (WPC-06). + * + * A failed hook never invalidates the create — the worktree is kept and the + * enclosing result stays `ok: true` (WPC-03). + */ +export interface PostCreateHookResult { + /** Whether the command exited 0. */ + ok: boolean + /** The command as declared in the repo's `.app/config.json`. */ + command: string + /** Exit code; -1 for a spawn failure or a timeout kill. */ + code: number + /** Combined stdout+stderr, last 4000 chars; '' when the command was silent. */ + output: string + /** Set only when the command was killed for exceeding the timeout (WPC-05). */ + timedOut?: boolean +} + /** Result of worktrees:create — failures are returned, never thrown. */ export interface CreateWorktreeResult { ok: boolean @@ -56,6 +77,12 @@ export interface CreateWorktreeResult { path?: string /** Human-readable failure message, present when ok is false. */ error?: string + /** + * The post-create hook's outcome (WPC-01). Present only when a worktree was + * created AND the repo declared a `postCreateCommand`; absent otherwise, which + * keeps the pre-feature result shape byte-identical (WPC-06). + */ + hook?: PostCreateHookResult /** * Set (with `ok: false` and no `error`) when a local branch of the requested * name already exists and the caller must choose to reuse or recreate it From bce57f419d92733f8a0dff0e4c1ebc165b85495d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 18:00:53 -0300 Subject: [PATCH 04/13] feat(worktree): run the post-create hook only when a worktree was created withPostCreateHook decorates a createWorktree-shaped function with an identical signature, so one instance wired in index.ts serves every call path and none can opt out. The hook runs iff the create reports ok with a path, which covers every no-worktree outcome (branch conflict, empty template, existing target, blocked refresh, git failure) without enumerating them, while the reuse and recreate successes do run it. A repo declaring no command comes back with no hook key at all. Covers WPC-01, WPC-03 (no-rollback), WPC-06 (no-hook-key), WPC-08, WPC-22. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/post-create-hook.test.ts | 184 +++++++++++++++++++++++++++++- src/main/post-create-hook.ts | 61 +++++++++- 2 files changed, 242 insertions(+), 3 deletions(-) diff --git a/src/main/post-create-hook.test.ts b/src/main/post-create-hook.test.ts index dd41888..f1e8370 100644 --- a/src/main/post-create-hook.test.ts +++ b/src/main/post-create-hook.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest' -import type { HookShell, HookShellResult } from './post-create-hook' -import { HOOK_OUTPUT_MAX_CHARS, HOOK_TIMEOUT_MS, runPostCreateHook } from './post-create-hook' +import type { CreateWorktreeResult } from '../shared/worktrees' +import type { CreateWorktreeFn, HookShell, HookShellResult } from './post-create-hook' +import { + HOOK_OUTPUT_MAX_CHARS, + HOOK_TIMEOUT_MS, + runPostCreateHook, + withPostCreateHook +} from './post-create-hook' interface ShellCall { cmd: string @@ -143,3 +149,177 @@ describe('runPostCreateHook', () => { expect(hook.ok).toBe(true) }) }) + +/** Records every create invocation so argument pass-through can be asserted. */ +function fakeCreate(result: CreateWorktreeResult): { create: CreateWorktreeFn; args: unknown[][] } { + const args: unknown[][] = [] + const create: CreateWorktreeFn = (...called) => { + args.push(called) + return Promise.resolve(result) + } + return { create, args } +} + +describe('withPostCreateHook', () => { + it('runs the command in the created worktree and attaches the outcome', async () => { + const { create } = fakeCreate({ ok: true, path: 'M:\\src\\Code-feature-x' }) + const { shell, calls } = fakeShell({ code: 0, stdout: 'junctions created' }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/x', 'main') + + expect(calls[0].cwd).toBe('M:\\src\\Code-feature-x') + expect(result.ok).toBe(true) + expect(result.path).toBe('M:\\src\\Code-feature-x') + expect(result.hook).toEqual({ + ok: true, + command: 'SetupSkills.cmd', + code: 0, + output: 'junctions created' + }) + }) + + it('keeps the worktree when the command fails', async () => { + const { create } = fakeCreate({ ok: true, path: 'M:\\src\\Code-feature-x' }) + const { shell } = fakeShell({ code: 1, stderr: 'ERRO' }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/x', 'main') + + expect(result.ok).toBe(true) + expect(result.path).toBe('M:\\src\\Code-feature-x') + expect(result.error).toBeUndefined() + expect(result.hook?.ok).toBe(false) + expect(result.hook?.code).toBe(1) + }) + + it('does not run the command when the branch already exists', async () => { + const { create } = fakeCreate({ ok: false, conflict: 'branch-exists' }) + const { shell, calls } = fakeShell({ code: 0 }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/x', 'main') + + expect(calls).toHaveLength(0) + expect('hook' in result).toBe(false) + expect(result.conflict).toBe('branch-exists') + }) + + it('does not run the command when the create failed', async () => { + const { create } = fakeCreate({ ok: false, error: 'Target path already exists: …' }) + const { shell, calls } = fakeShell({ code: 0 }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/x', 'main') + + expect(calls).toHaveLength(0) + expect('hook' in result).toBe(false) + }) + + it('does not run the command when the create reported no path', async () => { + const { create } = fakeCreate({ ok: true }) + const { shell, calls } = fakeShell({ code: 0 }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/x', 'main') + + expect(calls).toHaveLength(0) + expect('hook' in result).toBe(false) + }) + + it('runs the command on the reuse path', async () => { + const { create } = fakeCreate({ ok: true, path: 'M:\\src\\Code-feature-reuse' }) + const { shell, calls } = fakeShell({ code: 0 }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/reuse', 'main', undefined, false, 'reuse') + + expect(calls[0].cwd).toBe('M:\\src\\Code-feature-reuse') + expect(result.hook?.ok).toBe(true) + }) + + it('runs the command on the recreate path', async () => { + const { create } = fakeCreate({ ok: true, path: 'M:\\src\\Code-feature-re' }) + const { shell, calls } = fakeShell({ code: 0 }) + + const result = await withPostCreateHook(create, { + readCommand: () => 'SetupSkills.cmd', + shell + })('M:\\src\\Code', 'feature/re', 'main', undefined, false, 'recreate') + + expect(calls[0].cwd).toBe('M:\\src\\Code-feature-re') + expect(result.hook?.ok).toBe(true) + }) + + it('leaves the result untouched when the repo declares no command', async () => { + const { create } = fakeCreate({ ok: true, path: 'M:\\src\\Code-feature-x' }) + const { shell, calls } = fakeShell({ code: 0 }) + + const result = await withPostCreateHook(create, { readCommand: () => null, shell })( + 'M:\\src\\Code', + 'feature/x', + 'main' + ) + + expect(calls).toHaveLength(0) + expect('hook' in result).toBe(false) + expect(result).toEqual({ ok: true, path: 'M:\\src\\Code-feature-x' }) + }) + + it('forwards every create argument verbatim', async () => { + const { create, args } = fakeCreate({ ok: true, path: 'M:\\src\\Code-feature-x' }) + const { shell } = fakeShell({ code: 0 }) + + await withPostCreateHook(create, { readCommand: () => null, shell })( + 'M:\\src\\Code', + 'feature/x', + 'main', + '{repo}-{id}', + true, + 'reuse' + ) + + expect(args[0]).toEqual(['M:\\src\\Code', 'feature/x', 'main', '{repo}-{id}', true, 'reuse']) + }) + + it('keeps concurrent creates isolated from each other', async () => { + // Each shell call reports its own cwd back, delayed so the two interleave. + const shell: HookShell = (_cmd, opts) => + new Promise((resolve) => + setTimeout( + () => resolve({ code: 0, stdout: `ran in ${opts.cwd}`, stderr: '' }), + opts.cwd.endsWith('slow') ? 20 : 1 + ) + ) + const wrapSlow = withPostCreateHook( + fakeCreate({ ok: true, path: 'M:\\src\\Code-slow' }).create, + { readCommand: () => 'Slow.cmd', shell } + ) + const wrapFast = withPostCreateHook( + fakeCreate({ ok: true, path: 'M:\\src\\Code-fast' }).create, + { readCommand: () => 'Fast.cmd', shell } + ) + + const [slow, fast] = await Promise.all([ + wrapSlow('M:\\src\\Code', 'slow'), + wrapFast('M:\\src\\Code', 'fast') + ]) + + expect(slow.hook?.command).toBe('Slow.cmd') + expect(slow.hook?.output).toBe('ran in M:\\src\\Code-slow') + expect(fast.hook?.command).toBe('Fast.cmd') + expect(fast.hook?.output).toBe('ran in M:\\src\\Code-fast') + }) +}) diff --git a/src/main/post-create-hook.ts b/src/main/post-create-hook.ts index 4d774e5..0ecb62c 100644 --- a/src/main/post-create-hook.ts +++ b/src/main/post-create-hook.ts @@ -1,4 +1,4 @@ -import type { PostCreateHookResult } from '../shared/worktrees' +import type { CreateWorktreeResult, PostCreateHookResult } from '../shared/worktrees' /** How long a repo's init command may run before it is killed (WPC-05). */ export const HOOK_TIMEOUT_MS = 120000 @@ -79,3 +79,62 @@ function combinedTail({ stdout, stderr }: HookShellResult): string { const combined = [stdout, stderr].filter((stream) => stream !== '').join('\n') return combined.length > HOOK_OUTPUT_MAX_CHARS ? combined.slice(-HOOK_OUTPUT_MAX_CHARS) : combined } + +/** `createWorktree`'s exact shape — what the decorator consumes and returns. */ +export type CreateWorktreeFn = ( + repoPath: string, + branch: string, + baseBranch?: string, + worktreeTemplate?: string, + updateBase?: boolean, + onExisting?: 'reuse' | 'recreate' +) => Promise + +export interface PostCreateHookDeps { + /** Reads the repo's declared command; null when it declares none. */ + readCommand(repoPath: string): string | null + shell: HookShell +} + +/** + * Wraps a create with the repo's post-create hook, returning a function with the + * **identical signature** so it is a drop-in for every caller (WPC-10). Wiring + * this once in `index.ts` and handing it to both the IPC handler and the workflow + * ctx is what makes the hook non-optional — no call site can bypass it. + * + * The hook runs **iff the create actually produced a worktree**: `ok` with a + * `path`. That single test covers every no-worktree outcome without enumerating + * them (WPC-08) — a branch-exists conflict, an empty rendered template, an + * existing target path, a blocked base refresh and a failed `git worktree add` + * are all `ok: false` — while the successful `reuse`/`recreate` paths are + * `ok: true` with a path and so do run it. + * + * A repo that declares no command is returned untouched, with **no `hook` key at + * all**, keeping the pre-feature result shape byte-identical (WPC-06). A hook + * that fails never invalidates the create: `ok` and `path` pass through and the + * failure rides along in `hook` (WPC-03) — nothing here removes a worktree. + */ +export function withPostCreateHook( + create: CreateWorktreeFn, + deps: PostCreateHookDeps +): CreateWorktreeFn { + return async (repoPath, branch, baseBranch, worktreeTemplate, updateBase, onExisting) => { + const result = await create( + repoPath, + branch, + baseBranch, + worktreeTemplate, + updateBase, + onExisting + ) + if (!result.ok || typeof result.path !== 'string') return result + const command = deps.readCommand(repoPath) + if (command === null) return result + const hook = await runPostCreateHook( + command, + { worktreePath: result.path, repoPath, branch }, + deps.shell + ) + return { ...result, hook } + } +} From dd3eeaba40b8ffa2806fc3909ad84da8ca3ca929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 18:09:21 -0300 Subject: [PATCH 05/13] feat(worktree): wire the post-create hook into both create paths runHookShell spawns the repo's command through a shell so a checked-in .cmd works, capturing both streams and never throwing; spawn's own timeout kills it and the non-null close signal is what marks timedOut, since an exit code alone is ambiguous on Windows. One wrapped create is shared by the worktrees:create handler and the workflow ctx, so no call path can skip a repo's init command. Covers WPC-10 and WPC-05 (kill half). Co-Authored-By: Claude Opus 5 (1M context) --- src/main/index.ts | 52 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 75e5e49..72ad439 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -13,7 +13,9 @@ import { AgentStepRunner, type AgentChild, type AgentSpawn } from './agent-step- import { ConfigStore } from './config-store' import { emit, handle, onSend } from './ipc' import { createMcpResultServer } from './mcp-result-server' +import { withPostCreateHook, type HookShell, type HookShellResult } from './post-create-hook' import { PtyPort } from './pty-port' +import { repoPostCreateCommand } from './repo-config' import { SessionManager, type EmitFn } from './session-manager' import { ShortcutLauncher } from './shortcut-launcher' import { TaskBoard } from './task-board' @@ -79,6 +81,43 @@ function runShell(cmd: string, opts: { cwd: string }): Promise { }) } +/** + * WPC real `HookShell` seam (WPC-01): a repo's post-create command runs **through a + * shell** so a checked-in `.cmd`/`.ps1` works, in the new worktree, with the + * `PLAYGROUND_*` env the caller supplies. Same never-throw capture shape as + * `runShell` above — a spawn error becomes `code: -1` rather than a rejection. + * + * The timeout is `spawn`'s own: on expiry Node sends `killSignal`, and the `close` + * event then reports a non-null `signal`, which is the only reliable + * "killed-for-time" marker (an exit code alone is ambiguous on Windows). Only the + * spawned shell is killed — a detached grandchild can outlive it (documented Out + * of Scope; a real tree-kill is a separate concern). + */ +const runHookShell: HookShell = (cmd, { cwd, env, timeoutMs }) => { + return new Promise((resolve) => { + const child = spawn(cmd, { + cwd, + env, + shell: true, + windowsHide: true, + timeout: timeoutMs, + killSignal: 'SIGTERM' + }) + let stdout = '' + let stderr = '' + child.stdout?.on('data', (chunk) => (stdout += chunk.toString())) + child.stderr?.on('data', (chunk) => (stderr += chunk.toString())) + child.on('error', (err) => resolve({ code: -1, stdout, stderr: stderr + String(err) })) + child.on('close', (code, signal) => { + if (signal !== null) { + resolve({ code: -1, stdout, stderr, timedOut: true }) + return + } + resolve({ code: code ?? -1, stdout, stderr }) + }) + }) +} + /** * WF3 real `AgentSpawn` seam (WF3-01): spawn the resolved `claude` binary **directly** * (`shell:false`, argv verbatim) with **stdin closed** (`stdio:['ignore','pipe','pipe']`), @@ -186,10 +225,17 @@ app.whenReady().then(() => { handle('workspaces:remove', ({ id }) => registry.remove(id)) handle('workspaces:templates', ({ workspacePath }) => workspaceTemplates(workspacePath)) handle('tree:get', () => buildTree(registry)) + // WPC-10: ONE hook-wrapped create, shared by the IPC handler below and the + // workflow ctx further down. Because both consumers get this same wrapper — + // never bare `createWorktree` — no call path can skip a repo's init command. + const createWorktreeWithHook = withPostCreateHook(createWorktree, { + readCommand: repoPostCreateCommand, + shell: runHookShell + }) handle( 'worktrees:create', ({ repoPath, branch, baseBranch, worktreeTemplate, updateBase, onExisting }) => - createWorktree(repoPath, branch, baseBranch, worktreeTemplate, updateBase, onExisting) + createWorktreeWithHook(repoPath, branch, baseBranch, worktreeTemplate, updateBase, onExisting) ) handle('worktrees:remove', ({ repoPath, worktreePath, force }) => removeWorktree(repoPath, worktreePath, { force }) @@ -297,7 +343,9 @@ app.whenReady().then(() => { const workflowsAdo = new AdoGateway() const ctxDeps: CtxDeps = { worktree: { - create: createWorktree, + // The hook-wrapped create (WPC-10) — a workflow-created worktree for an + // agent is the case that most needs the repo's init command to have run. + create: createWorktreeWithHook, remove: removeWorktree, changedFiles: changedFilesOf }, From 0ce617733947a6b25fbc2f6c1d01327f6ce54df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 18:11:20 -0300 Subject: [PATCH 06/13] feat(worktree): add hook failure notice component Footer-region report for a failed post-create command, same geometry as BranchExistsChoice and the same evidence layout as the workflow failed footer, but amber rather than red: the create succeeded and the worktree is kept, so this is advisory. Shows the created path, the command, exit code or a timeout label, and the output tail (omitted entirely when empty). Covers WPC-12/13 (presentation) and WPC-14 (action). Co-Authored-By: Claude Opus 5 (1M context) --- .../src/components/HookFailureNotice.css | 74 +++++++++++++++++++ .../src/components/HookFailureNotice.tsx | 53 +++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 src/renderer/src/components/HookFailureNotice.css create mode 100644 src/renderer/src/components/HookFailureNotice.tsx diff --git a/src/renderer/src/components/HookFailureNotice.css b/src/renderer/src/components/HookFailureNotice.css new file mode 100644 index 0000000..77150d2 --- /dev/null +++ b/src/renderer/src/components/HookFailureNotice.css @@ -0,0 +1,74 @@ +/* + * Post-create hook failure report (WPC-12/13) — lives in the dialog footer + * region, same geometry as .branch-exists. Amber rather than red: the worktree + * was created and kept, only its init command failed. + */ + +.hook-failure { + padding: 16px 24px; + border-top: 1px solid var(--border); + display: flex; + flex-direction: column; + gap: 8px; +} + +.hook-failure-head { + display: flex; + align-items: center; + gap: 8px; + font-size: 13.5px; + font-weight: 700; + color: var(--amber); +} + +.hook-failure-path { + font-family: var(--font-mono); + font-size: 12px; + color: var(--text); + word-break: break-all; +} + +.hook-failure-cmd { + font-family: var(--font-mono); + font-size: 12px; + color: var(--text-muted); + background: color-mix(in oklab, var(--amber) 12%, transparent); + border-radius: 6px; + padding: 4px 8px; + align-self: flex-start; + word-break: break-word; +} + +.hook-failure-code { + font-family: var(--font-mono); + font-size: 12px; + color: var(--amber); +} + +.hook-failure-output { + font-family: var(--font-mono); + font-size: 11.5px; + line-height: 1.5; + color: var(--text-muted); + white-space: pre-wrap; + word-break: break-word; + max-height: 180px; + overflow-y: auto; + margin: 0; + padding: 10px 12px; + border: 1px solid var(--border); + border-radius: 10px; + background: var(--panel-2); +} + +.hook-failure-note { + font-size: 11.5px; + line-height: 1.5; + color: var(--text-faint); +} + +.hook-failure-actions { + display: flex; + justify-content: flex-end; + gap: 10px; +} diff --git a/src/renderer/src/components/HookFailureNotice.tsx b/src/renderer/src/components/HookFailureNotice.tsx new file mode 100644 index 0000000..b1f619e --- /dev/null +++ b/src/renderer/src/components/HookFailureNotice.tsx @@ -0,0 +1,53 @@ +import type { JSX } from 'react' +import type { PostCreateHookResult } from '../../../shared/worktrees' +import { Icon } from './Icon' +import './HookFailureNotice.css' + +interface HookFailureNoticeProps { + /** Absolute path of the worktree that WAS created (WPC-12/13). */ + worktreePath: string + hook: PostCreateHookResult + /** Dismisses the notice and continues the normal post-create flow (WPC-14). */ + onProceed: () => void +} + +/** + * Inline report for a post-create command that failed (WPC-12/13), shown in the + * dialog's footer region like `BranchExistsChoice`. Deliberately **amber, not + * red**: the create itself succeeded and the worktree is kept (WPC-03) — only its + * initialization fell over, so this is an advisory, not a failure. + * + * Leads with the created path so the outcome is unambiguous, then the evidence: + * the command, how it ended, and the captured output tail. The single action + * continues into the normal post-create flow rather than offering an undo (WPC-14), + * and the create button is not rendered while this is up, so the same create can + * never be re-submitted (WPC-16). + */ +export function HookFailureNotice({ + worktreePath, + hook, + onProceed +}: HookFailureNoticeProps): JSX.Element { + return ( +
+
+ + Worktree created, but its init command failed +
+
{worktreePath}
+
{hook.command}
+
+ {hook.timedOut ? 'timed out and was stopped' : `exit code ${hook.code}`} +
+ {hook.output !== '' && 
{hook.output}
} +
+ The worktree was kept — run the command yourself in it once the cause is fixed. +
+
+ +
+
+ ) +} From cd95f5abfe95c6c0fa57ace39ee620e7b322af8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 18:16:22 -0300 Subject: [PATCH 07/13] feat(worktree): surface post-create hook failures in the create dialogs Both create dialogs hold a hookFailure state and swap the footer for the advisory when a created worktree's init command failed, mirroring how the branch-exists choice already takes over that region. Continue calls the same onCreated the happy path calls, so the worktree is never stranded behind an error, and because the footer is replaced the create button cannot re-submit. A successful or absent hook leaves today's behaviour untouched. Covers WPC-12, WPC-13, WPC-14, WPC-15, WPC-16. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/components/NewWorktreeDialog.tsx | 24 ++++++++++++++++++- .../src/components/StartWorkDialog.tsx | 24 ++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/NewWorktreeDialog.tsx b/src/renderer/src/components/NewWorktreeDialog.tsx index 1d44077..2b78f48 100644 --- a/src/renderer/src/components/NewWorktreeDialog.tsx +++ b/src/renderer/src/components/NewWorktreeDialog.tsx @@ -1,10 +1,12 @@ import { useEffect, useState } from 'react' import type { JSX } from 'react' import type { WorkspaceNode } from '../../../shared/tree' +import type { PostCreateHookResult } from '../../../shared/worktrees' import { worktreePathFor } from '../../../shared/worktrees' import { api } from '../lib/api' import { defaultBaseFor, repoOptionsOf } from '../lib/repo-options' import { BranchExistsChoice } from './BranchExistsChoice' +import { HookFailureNotice } from './HookFailureNotice' import { Icon } from './Icon' import './NewWorktreeDialog.css' @@ -40,6 +42,13 @@ export function NewWorktreeDialog({ // Set when create reports the branch already exists — swaps the footer for the // reuse/recreate choice (EXB-06). const [conflict, setConflict] = useState<'branch-exists' | null>(null) + // Set when the worktree was created but the repo's init command failed — swaps + // the footer for the advisory (WPC-12). Holds the created path so Continue can + // proceed with the normal flow (WPC-14). + const [hookFailure, setHookFailure] = useState<{ + path: string + hook: PostCreateHookResult + } | null>(null) const repoOptions = repoOptionsOf(tree) const selectedRepo = repoOptions.find((r) => r.path === repoPath) @@ -91,6 +100,13 @@ export function NewWorktreeDialog({ }) .then((result) => { if (result.ok && result.path) { + // The worktree exists either way; a failed init command only earns an + // advisory before the normal flow continues (WPC-12/15). + if (result.hook && !result.hook.ok) { + setHookFailure({ path: result.path, hook: result.hook }) + setBusy(false) + return + } onCreated(result.path) return } @@ -187,7 +203,13 @@ export function NewWorktreeDialog({ )} - {conflict ? ( + {hookFailure ? ( + onCreated(hookFailure.path)} + /> + ) : conflict ? ( (null) + // Set when the worktree was created but the repo's init command failed — swaps + // the footer for the advisory (WPC-13). Holds the created path so Continue can + // proceed with the normal flow (WPC-14). + const [hookFailure, setHookFailure] = useState<{ + path: string + hook: PostCreateHookResult + } | null>(null) const branchEdited = useRef(false) const selectedRepo = repoOptions.find((r) => r.path === repoPath) @@ -108,6 +117,13 @@ export function StartWorkDialog({ }) .then((result) => { if (result.ok && result.path) { + // The worktree exists either way; a failed init command only earns an + // advisory before the normal flow continues (WPC-13/15). + if (result.hook && !result.hook.ok) { + setHookFailure({ path: result.path, hook: result.hook }) + setBusy(false) + return + } onCreated(result.path) return } @@ -218,7 +234,13 @@ export function StartWorkDialog({ )} - {conflict ? ( + {hookFailure ? ( + onCreated(hookFailure.path)} + /> + ) : conflict ? ( Date: Wed, 29 Jul 2026 18:40:06 -0300 Subject: [PATCH 08/13] fix(worktree): settle the hook shell on exit so a hung command can't stall a create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving on `close` alone waits for every inherited pipe to reach EOF, and spawn's timeout kills only cmd.exe — a surviving grandchild holds those pipes open. Measured: `ping -n 12` with timeout 1500ms exits at 1665ms but closes only at 12969ms; with a `start /b` grandchild, 21000ms. For a genuinely hung script `close` never arrives, so worktrees:create never resolved and the dialog stuck on busy forever. Now whichever comes first wins: close (complete output, the normal path) or exit plus a short flush grace period (guaranteed progress). Extracted from index.ts into hook-shell.ts so the settle condition carries real-process tests — a fake shell cannot prove a settle condition. Reverting the fix makes the two timeout tests fail at 5018ms and 9244ms. Found by the independent Verifier; the previous comment overstated the guarantee. Covers WPC-05 (kill half). Co-Authored-By: Claude Opus 5 (1M context) --- src/main/hook-shell.test.ts | 95 +++++++++++++++++++++++++++++++++++++ src/main/hook-shell.ts | 69 +++++++++++++++++++++++++++ src/main/index.ts | 40 +--------------- 3 files changed, 166 insertions(+), 38 deletions(-) create mode 100644 src/main/hook-shell.test.ts create mode 100644 src/main/hook-shell.ts diff --git a/src/main/hook-shell.test.ts b/src/main/hook-shell.test.ts new file mode 100644 index 0000000..1737e36 --- /dev/null +++ b/src/main/hook-shell.test.ts @@ -0,0 +1,95 @@ +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { runHookShell } from './hook-shell' + +/** + * Real-process tests for the one I/O seam (the repo's real-temp-dir pattern, no + * mocks). They exist because the seam's contract is a settle *condition*, which is + * exactly what a fake shell cannot prove. + */ +describe('runHookShell', () => { + let dir: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'wtm-hookshell-')) + }) + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + const env = { ...process.env } + + it('runs the command through a shell in the given cwd and captures stdout', async () => { + const result = await runHookShell('echo hello-from-hook', { cwd: dir, env, timeoutMs: 30000 }) + + expect(result.code).toBe(0) + expect(result.stdout).toContain('hello-from-hook') + expect(result.timedOut).toBeUndefined() + }) + + it('reports the exit code of a failing command', async () => { + const result = await runHookShell('exit 3', { cwd: dir, env, timeoutMs: 30000 }) + + expect(result.code).toBe(3) + expect(result.timedOut).toBeUndefined() + }) + + it('passes the environment through to the command', async () => { + const result = await runHookShell('echo %PLAYGROUND_BRANCH%', { + cwd: dir, + env: { ...env, PLAYGROUND_BRANCH: 'feature/from-env' }, + timeoutMs: 30000 + }) + + expect(result.stdout).toContain('feature/from-env') + }) + + it('executes in the worktree directory', async () => { + const result = await runHookShell('cd', { cwd: dir, env, timeoutMs: 30000 }) + + // `cd` with no argument prints the current directory on Windows shells. + expect(result.stdout.trim().toLowerCase()).toContain(dir.toLowerCase()) + }) + + it('lets a shell command write into the worktree', async () => { + await runHookShell('echo marker > initialized.txt', { cwd: dir, env, timeoutMs: 30000 }) + + expect(readFileSync(join(dir, 'initialized.txt'), 'utf8')).toContain('marker') + }) + + // The two timeout tests deliberately run in tmpdir() rather than the per-test + // directory: killing the shell does not kill its children, and a surviving + // grandchild holds its cwd open, which would make the afterEach cleanup fail + // with EPERM on Windows. The cwd is irrelevant to what they assert. + it('kills a long-running command and flags it as timed out', async () => { + const result = await runHookShell('ping -n 10 127.0.0.1', { + cwd: tmpdir(), + env, + timeoutMs: 700 + }) + + expect(result.timedOut).toBe(true) + expect(result.code).toBe(-1) + }) + + it('returns promptly when a surviving grandchild holds the pipes open', async () => { + // Regression guard: settling on `close` alone waits for every inherited pipe + // to reach EOF. Killing the shell does not kill its children, so a grandchild + // keeps those pipes open and `close` lags the kill by many seconds — or never + // arrives. This must still return, and must still say it timed out. + const started = Date.now() + const result = await runHookShell('start /b ping -n 10 127.0.0.1 & ping -n 10 127.0.0.1', { + cwd: tmpdir(), + env, + timeoutMs: 700 + }) + const elapsed = Date.now() - started + + expect(result.timedOut).toBe(true) + expect(result.code).toBe(-1) + expect(elapsed).toBeLessThan(6000) + }, 20000) +}) diff --git a/src/main/hook-shell.ts b/src/main/hook-shell.ts new file mode 100644 index 0000000..7b95c8b --- /dev/null +++ b/src/main/hook-shell.ts @@ -0,0 +1,69 @@ +import { spawn } from 'node:child_process' +import type { HookShell, HookShellResult } from './post-create-hook' + +/** + * How long to keep waiting for stdio to flush after the shell has exited before + * settling anyway. `close` (all stdio closed) carries the complete output and + * normally fires within a millisecond of `exit`, so the happy path never waits + * this long — it exists purely so a surviving grandchild holding the inherited + * pipes open cannot stall the result forever. + */ +export const HOOK_FLUSH_GRACE_MS = 250 + +/** + * The real `HookShell` (WPC-01): runs a repo's post-create command **through a + * shell**, so a checked-in `.cmd`/`.ps1` works, in the new worktree with the + * `PLAYGROUND_*` env the caller supplies. Never throws — a spawn error becomes + * `code: -1` rather than a rejection. + * + * **Settling is deliberately not just `close`.** The timeout is `spawn`'s own: on + * expiry Node signals the shell, and a non-null `signal` is the only reliable + * "killed for time" marker (an exit code alone is ambiguous on Windows). But + * `close` waits for every inherited pipe to reach EOF, and killing `cmd.exe` does + * NOT kill its children — a surviving grandchild holds those pipes open, so `close` + * can lag the kill by many seconds or never arrive at all. Waiting on it alone + * would leave `worktrees:create` unresolved and the caller stuck. + * + * So whichever comes first wins: `close` (complete output, the normal case) or + * `exit` plus a short flush grace period (guaranteed progress). Either way the + * result is returned promptly. A detached grandchild may still outlive the shell — + * that limitation stands (a real process-tree kill is out of scope) — but it can no + * longer hold the create hostage. + */ +export const runHookShell: HookShell = (cmd, { cwd, env, timeoutMs }) => { + return new Promise((resolve) => { + const child = spawn(cmd, { + cwd, + env, + shell: true, + windowsHide: true, + timeout: timeoutMs, + killSignal: 'SIGTERM' + }) + let stdout = '' + let stderr = '' + let settled = false + let graceTimer: NodeJS.Timeout | undefined + + const settle = (result: HookShellResult): void => { + if (settled) return + settled = true + if (graceTimer) clearTimeout(graceTimer) + resolve(result) + } + const outcome = (code: number | null, signal: NodeJS.Signals | null): HookShellResult => + signal !== null + ? { code: -1, stdout, stderr, timedOut: true } + : { code: code ?? -1, stdout, stderr } + + child.stdout?.on('data', (chunk) => (stdout += chunk.toString())) + child.stderr?.on('data', (chunk) => (stderr += chunk.toString())) + child.on('error', (err) => settle({ code: -1, stdout, stderr: stderr + String(err) })) + child.on('close', (code, signal) => settle(outcome(code, signal))) + child.on('exit', (code, signal) => { + graceTimer = setTimeout(() => settle(outcome(code, signal)), HOOK_FLUSH_GRACE_MS) + // Don't hold the event loop open just for the grace timer. + graceTimer.unref?.() + }) + }) +} diff --git a/src/main/index.ts b/src/main/index.ts index 72ad439..58d31b0 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -11,9 +11,10 @@ import icon from '../../resources/icon.png?asset' import { AdoGateway } from './ado-gateway' import { AgentStepRunner, type AgentChild, type AgentSpawn } from './agent-step-runner' import { ConfigStore } from './config-store' +import { runHookShell } from './hook-shell' import { emit, handle, onSend } from './ipc' import { createMcpResultServer } from './mcp-result-server' -import { withPostCreateHook, type HookShell, type HookShellResult } from './post-create-hook' +import { withPostCreateHook } from './post-create-hook' import { PtyPort } from './pty-port' import { repoPostCreateCommand } from './repo-config' import { SessionManager, type EmitFn } from './session-manager' @@ -81,43 +82,6 @@ function runShell(cmd: string, opts: { cwd: string }): Promise { }) } -/** - * WPC real `HookShell` seam (WPC-01): a repo's post-create command runs **through a - * shell** so a checked-in `.cmd`/`.ps1` works, in the new worktree, with the - * `PLAYGROUND_*` env the caller supplies. Same never-throw capture shape as - * `runShell` above — a spawn error becomes `code: -1` rather than a rejection. - * - * The timeout is `spawn`'s own: on expiry Node sends `killSignal`, and the `close` - * event then reports a non-null `signal`, which is the only reliable - * "killed-for-time" marker (an exit code alone is ambiguous on Windows). Only the - * spawned shell is killed — a detached grandchild can outlive it (documented Out - * of Scope; a real tree-kill is a separate concern). - */ -const runHookShell: HookShell = (cmd, { cwd, env, timeoutMs }) => { - return new Promise((resolve) => { - const child = spawn(cmd, { - cwd, - env, - shell: true, - windowsHide: true, - timeout: timeoutMs, - killSignal: 'SIGTERM' - }) - let stdout = '' - let stderr = '' - child.stdout?.on('data', (chunk) => (stdout += chunk.toString())) - child.stderr?.on('data', (chunk) => (stderr += chunk.toString())) - child.on('error', (err) => resolve({ code: -1, stdout, stderr: stderr + String(err) })) - child.on('close', (code, signal) => { - if (signal !== null) { - resolve({ code: -1, stdout, stderr, timedOut: true }) - return - } - resolve({ code: code ?? -1, stdout, stderr }) - }) - }) -} - /** * WF3 real `AgentSpawn` seam (WF3-01): spawn the resolved `claude` binary **directly** * (`shell:false`, argv verbatim) with **stdin closed** (`stdio:['ignore','pipe','pipe']`), From adff2bbb3702270c78fcaa17379fa815bae80531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 18:40:08 -0300 Subject: [PATCH 09/13] test(worktree): pin the output bound and prove the worktree survives a failed hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WPC-11's 4000-char bound was only asserted as toHaveLength(HOOK_OUTPUT_MAX_CHARS), which is self-referential — a mutation of the constant survived. Now pinned to the spec's literal. WPC-03's "the worktree still exists on disk" rested on a structural argument (no deletion code exists) with no automated evidence. Added the spec's own Independent Test end to end over real git, a real .app/config.json and the real shell: the command runs inside the new worktree, a failing command leaves the worktree and its files in place, and a repo declaring nothing yields no hook key. Both gaps raised by the independent Verifier. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/post-create-hook.test.ts | 85 ++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/src/main/post-create-hook.test.ts b/src/main/post-create-hook.test.ts index f1e8370..d088cf2 100644 --- a/src/main/post-create-hook.test.ts +++ b/src/main/post-create-hook.test.ts @@ -1,5 +1,10 @@ -import { describe, expect, it } from 'vitest' +import { execFileSync } from 'node:child_process' +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' import type { CreateWorktreeResult } from '../shared/worktrees' +import { runHookShell } from './hook-shell' import type { CreateWorktreeFn, HookShell, HookShellResult } from './post-create-hook' import { HOOK_OUTPUT_MAX_CHARS, @@ -7,6 +12,8 @@ import { runPostCreateHook, withPostCreateHook } from './post-create-hook' +import { repoPostCreateCommand } from './repo-config' +import { createWorktree } from './worktree-manager' interface ShellCall { cmd: string @@ -121,7 +128,11 @@ describe('runPostCreateHook', () => { const hook = await runPostCreateHook('SetupSkills.cmd', ctx, shell) - expect(hook.output).toHaveLength(HOOK_OUTPUT_MAX_CHARS) + // Pinned to the spec's literal bound, not to the constant — asserting + // toHaveLength(HOOK_OUTPUT_MAX_CHARS) alone is self-referential and would + // still pass if the bound were changed. + expect(HOOK_OUTPUT_MAX_CHARS).toBe(4000) + expect(hook.output).toHaveLength(4000) expect(hook.output.endsWith('TAIL')).toBe(true) }) @@ -323,3 +334,73 @@ describe('withPostCreateHook', () => { expect(fast.hook?.output).toBe('ran in M:\\src\\Code-fast') }) }) + +/** + * The spec's own Independent Test for WPC-01/WPC-03, wired end to end over real + * git, a real repo config and a real shell — the only way to assert the part that + * matters most: a failed init command must not cost you the worktree ON DISK. + */ +describe('withPostCreateHook over real git', () => { + let root: string + let repo: string + + const git = (cwd: string, ...args: string[]): void => { + execFileSync('git', args, { cwd, windowsHide: true }) + } + + const declareCommand = (command: string): void => { + mkdirSync(join(repo, '.app'), { recursive: true }) + writeFileSync(join(repo, '.app', 'config.json'), JSON.stringify({ postCreateCommand: command })) + } + + const create = withPostCreateHook(createWorktree, { + readCommand: repoPostCreateCommand, + shell: runHookShell + }) + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'wtm-hookint-')) + repo = join(root, 'repo') + mkdirSync(repo) + git(repo, 'init', '-b', 'main') + git(repo, 'config', 'user.email', 'test@test.local') + git(repo, 'config', 'user.name', 'Test') + writeFileSync(join(repo, 'a.txt'), 'one', 'utf8') + git(repo, 'add', '.') + git(repo, 'commit', '-m', 'init') + }) + + afterEach(() => { + rmSync(root, { recursive: true, force: true }) + }) + + it('runs the declared command inside the new worktree', async () => { + declareCommand('echo initialized > hook-ran.txt') + + const result = await create(repo, 'feature/ok', 'main') + + expect(result.ok).toBe(true) + expect(result.hook?.ok).toBe(true) + expect(existsSync(join(result.path as string, 'hook-ran.txt'))).toBe(true) + }) + + it('keeps the worktree on disk when the command fails', async () => { + declareCommand('exit 1') + + const result = await create(repo, 'feature/bad', 'main') + + expect(result.ok).toBe(true) + expect(result.hook?.ok).toBe(false) + expect(result.hook?.code).toBe(1) + expect(existsSync(result.path as string)).toBe(true) + expect(existsSync(join(result.path as string, 'a.txt'))).toBe(true) + }) + + it('leaves no hook on the result when the repo declares nothing', async () => { + const result = await create(repo, 'feature/plain', 'main') + + expect(result.ok).toBe(true) + expect('hook' in result).toBe(false) + expect(existsSync(result.path as string)).toBe(true) + }) +}) From 98034ebb7f34f9d128da5037b326d32f68194189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 18:40:09 -0300 Subject: [PATCH 10/13] fix(worktree): keep the tree refresh when the hook advisory is dismissed by backdrop Clicking the backdrop while the advisory was up called onClose, skipping the refresh-and-select the Continue button performs, so a worktree that really existed didn't appear in the tree until the next refresh. Both dismissal paths now continue the post-create flow (WPC-14). Co-Authored-By: Claude Opus 5 (1M context) --- src/renderer/src/components/NewWorktreeDialog.tsx | 7 ++++++- src/renderer/src/components/StartWorkDialog.tsx | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/NewWorktreeDialog.tsx b/src/renderer/src/components/NewWorktreeDialog.tsx index 2b78f48..3bc5c45 100644 --- a/src/renderer/src/components/NewWorktreeDialog.tsx +++ b/src/renderer/src/components/NewWorktreeDialog.tsx @@ -125,7 +125,12 @@ export function NewWorktreeDialog({ } return ( -
+ // While the hook advisory is up the worktree already exists, so dismissing by + // backdrop must continue the post-create flow, not silently drop it (WPC-14). +
onCreated(hookFailure.path) : onClose} + >
event.stopPropagation()}>
New worktree
diff --git a/src/renderer/src/components/StartWorkDialog.tsx b/src/renderer/src/components/StartWorkDialog.tsx index 725ec16..b059a20 100644 --- a/src/renderer/src/components/StartWorkDialog.tsx +++ b/src/renderer/src/components/StartWorkDialog.tsx @@ -142,7 +142,12 @@ export function StartWorkDialog({ } return ( -
+ // While the hook advisory is up the worktree already exists, so dismissing by + // backdrop must continue the post-create flow, not silently drop it (WPC-14). +
onCreated(hookFailure.path) : onClose} + >
event.stopPropagation()}>
Start work
From 663e2d34930a0bfa79e9cc472382aa2fa751bdb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 19:33:22 -0300 Subject: [PATCH 11/13] fix(worktree): make the hook shell's grace timer actually guarantee progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grace timer was unref'd, so it could not keep the event loop alive on its own — "guaranteed progress" only held because close or the open pipes kept the process running. A scratch variant with the close handler removed exits with the promise unsettled, proving the two paths were not independent. The timer is now referenced and the doc comment no longer overstates the guarantee. Also closes the sensor gap on the real seam: stderr capture was asserted nowhere (the runner's stderr tests all drive a fake shell), and nothing pinned that a killed command still reports the output it produced before the kill, which is what WPC-05 specifies and what the grace period exists for. Adds a large-burst test (2000 lines) guarding against truncation on the exit path. Shortened the two lingering ping grandchildren from ~10s to ~4s with a tightened discrimination bound, so these tests stop pushing the pre-existing real-git suite over its 5000ms default timeout. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/hook-shell.test.ts | 45 ++++++++++++++++++++++++++++++++----- src/main/hook-shell.ts | 21 ++++++++++++----- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/main/hook-shell.test.ts b/src/main/hook-shell.test.ts index 1737e36..c429dd2 100644 --- a/src/main/hook-shell.test.ts +++ b/src/main/hook-shell.test.ts @@ -37,6 +37,35 @@ describe('runHookShell', () => { expect(result.timedOut).toBeUndefined() }) + it('captures stderr from a failing command', async () => { + // The real seam's stderr wiring is asserted nowhere else — the runner's stderr + // tests all drive a fake shell, so only this pins the actual capture. + const result = await runHookShell('echo bad-news 1>&2 & exit 1', { + cwd: dir, + env, + timeoutMs: 30000 + }) + + expect(result.code).toBe(1) + expect(result.stderr).toContain('bad-news') + }) + + it('captures a large burst of output in full', async () => { + // Guards the settle race in the direction the timeout tests cannot: settling on + // `exit` must never truncate a command that is still flushing. 2000 lines is far + // more than a pipe buffer holds. + const result = await runHookShell('for /L %i in (1,1,2000) do @echo line-%i', { + cwd: dir, + env, + timeoutMs: 30000 + }) + + expect(result.code).toBe(0) + expect(result.stdout).toContain('line-1\r\n') + expect(result.stdout).toContain('line-2000') + expect(result.stdout.split('line-').length - 1).toBe(2000) + }) + it('passes the environment through to the command', async () => { const result = await runHookShell('echo %PLAYGROUND_BRANCH%', { cwd: dir, @@ -65,14 +94,18 @@ describe('runHookShell', () => { // grandchild holds its cwd open, which would make the afterEach cleanup fail // with EPERM on Windows. The cwd is irrelevant to what they assert. it('kills a long-running command and flags it as timed out', async () => { - const result = await runHookShell('ping -n 10 127.0.0.1', { + const result = await runHookShell('ping -n 5 127.0.0.1', { cwd: tmpdir(), env, - timeoutMs: 700 + timeoutMs: 500 }) expect(result.timedOut).toBe(true) expect(result.code).toBe(-1) + // WPC-05 keeps the output captured *so far* — the flush grace period is what + // makes that true on the kill path, where `close` never arrives in time. + // (Asserted on the address, not on ping's localized prose.) + expect(result.stdout).toContain('127.0.0.1') }) it('returns promptly when a surviving grandchild holds the pipes open', async () => { @@ -81,15 +114,17 @@ describe('runHookShell', () => { // keeps those pipes open and `close` lags the kill by many seconds — or never // arrives. This must still return, and must still say it timed out. const started = Date.now() - const result = await runHookShell('start /b ping -n 10 127.0.0.1 & ping -n 10 127.0.0.1', { + const result = await runHookShell('start /b ping -n 5 127.0.0.1 & ping -n 5 127.0.0.1', { cwd: tmpdir(), env, - timeoutMs: 700 + timeoutMs: 500 }) const elapsed = Date.now() - started expect(result.timedOut).toBe(true) expect(result.code).toBe(-1) - expect(elapsed).toBeLessThan(6000) + // Settling on `exit` lands ~750ms; waiting for `close` would take ~4s while the + // grandchild holds the pipes. The bound has to sit between the two to discriminate. + expect(elapsed).toBeLessThan(2000) }, 20000) }) diff --git a/src/main/hook-shell.ts b/src/main/hook-shell.ts index 7b95c8b..1945649 100644 --- a/src/main/hook-shell.ts +++ b/src/main/hook-shell.ts @@ -25,10 +25,19 @@ export const HOOK_FLUSH_GRACE_MS = 250 * would leave `worktrees:create` unresolved and the caller stuck. * * So whichever comes first wins: `close` (complete output, the normal case) or - * `exit` plus a short flush grace period (guaranteed progress). Either way the - * result is returned promptly. A detached grandchild may still outlive the shell — - * that limitation stands (a real process-tree kill is out of scope) — but it can no - * longer hold the create hostage. + * `exit` plus a short flush grace period (progress even when `close` is hostage to + * a grandchild). Either way the result is returned promptly. A detached grandchild + * may still outlive the shell — that limitation stands (a real process-tree kill is + * out of scope) — but it can no longer hold the create hostage. + * + * The grace timer is deliberately **not** `unref`'d: it must be able to keep the + * event loop alive on its own for those few milliseconds, or the `exit` path would + * only appear to guarantee progress while actually depending on `close`/the open + * pipes to keep the process running. + * + * Output completeness is unaffected. A process cannot exit until its writes have + * been accepted, so at most a pipe-buffer's worth is in flight at `exit`, and + * `close` wins that race — measured complete for a 1 MB single burst. */ export const runHookShell: HookShell = (cmd, { cwd, env, timeoutMs }) => { return new Promise((resolve) => { @@ -61,9 +70,9 @@ export const runHookShell: HookShell = (cmd, { cwd, env, timeoutMs }) => { child.on('error', (err) => settle({ code: -1, stdout, stderr: stderr + String(err) })) child.on('close', (code, signal) => settle(outcome(code, signal))) child.on('exit', (code, signal) => { + // Left referenced on purpose (see the note above): this timer is the only + // thing guaranteeing the promise settles when `close` is held hostage. graceTimer = setTimeout(() => settle(outcome(code, signal)), HOOK_FLUSH_GRACE_MS) - // Don't hold the event loop open just for the grace timer. - graceTimer.unref?.() }) }) } From 8b2268e71be35f0ef7acced0dc95ac1b60c2c189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Bogoni?= Date: Wed, 29 Jul 2026 19:36:33 -0300 Subject: [PATCH 12/13] docs(worktree): record post-create hook validation, statuses and usage Validation report (Verifier rounds 1-2), execution record with the deviation that moved the shell seam into its own tested module, and the traceability table flipped to Verified for the 21 P1 requirements. WPC-12..16 are marked "Built" rather than Verified: the renderer has no unit tests by convention and no visual pass has been run. STATE.md handoff replaced (decisions log untouched) with the commit map, the accepted equivalent-mutant survivors, and the next step. README documents the new repo-local postCreateCommand, including the JSON-escaped ".\\" form and the repo-content trust trade-off. Co-Authored-By: Claude Opus 5 (1M context) --- .specs/STATE.md | 108 +-- .../worktree-post-create-hook/spec.md | 138 ++-- .../worktree-post-create-hook/tasks.md | 29 +- .../worktree-post-create-hook/validation.md | 638 ++++++++++++++++++ README.md | 21 + 5 files changed, 824 insertions(+), 110 deletions(-) create mode 100644 .specs/features/worktree-post-create-hook/validation.md diff --git a/.specs/STATE.md b/.specs/STATE.md index 1afa38a..f04ce45 100644 --- a/.specs/STATE.md +++ b/.specs/STATE.md @@ -23,58 +23,74 @@ Handoff snapshot. ## Handoff -**Status (current, 2026-07-06):** **Workflows epic (issue #56) — DONE + CLOSED.** WF1–WF5 -(incl. the WF5 hi-fi rebuild) all **MERGED to `main`**. WF5 + hifi landed via **PR #67** -(merge commit `668b2d4`); the `feature/workflows-ui` branch is deleted; issue #56 is **CLOSED**. -The owner-run two-example UI gate **PASSED** (handoff fidelity confirmed). Details of the hifi -slice below for the record. +**Status (current, 2026-07-29):** **`worktree-post-create-hook` (AD-013) — EXECUTED + +VERIFIED (round 2 PASS) — NOT pushed, NO PR, visual/UAT pass OUTSTANDING.** Branch +`feature/worktree-post-create-hook`, 11 commits (`c846eb0..663e2d3`), **533 tests / 39 files +green**, typecheck + lint (18 pre-existing warnings) + `build` + `build:win` all clean. -**WF5 hi-fi rebuild (`workflows-ui-hifi`, AD-012): EXECUTED + VERIFIED (PASS) + MERGED.** Built on -the (now deleted) branch `feature/workflows-ui`. All 11 tasks / 5 phases committed inline via one sub-agent per phase -(`d256870..c38f996`, 11 atomic commits). **Verifier PASS** (independent, author ≠ verifier): -10/10 backend unit ACs matched spec outcome (payload/conjunction rule satisfied — every field -asserted on value), 14/14 renderer ACs data-path present (visual proof deferred to the owner UI -gate per convention), gate green **486/486 tests / 36 files**, `npm run build` OK, discrimination -sensor **6/6 mutants killed** (reducer step-finished guard, manager durationMs, stepId -monotonicity, agent `read` permission default, groupRollup precedence, stepStatus ok-flag), no -survivors, no gaps. Report: `.specs/features/workflows-ui-hifi/validation.md`. **Merged to `main`** -via PR #67 (`668b2d4`, `Closes #56`, admin merge per the copilot_code_review ruleset). +A repo declares `postCreateCommand` in its own NEW `\.app\config.json`; it runs with +cwd = the new worktree on **all three create paths**, via a `withPostCreateHook` decorator +wired once in `index.ts` and handed to both the IPC handler and `ctxDeps.worktree.create` +(so no caller can opt out). A failed hook keeps the worktree and reports exit code + a +4000-char output tail; the dialogs show an amber advisory. `worktree-manager.ts` and +`workflow-ctx.ts` were never touched. -**Commit map (`d256870..c38f996`, in order):** +**Commit map:** | Commit | Task | What | | ------ | ---- | ---- | -| d256870 | T1 | StepEvent enrichment (StepKind/PermissionPreset/StepDetail, `step-finished` kind + stepId/stepKind/durationMs/ok/agent/agentResult/detail) + ipc `workflow:run-started`/blocked `sessionId?` + reducer `step-finished` fold (+4 tests) | -| ddba92b | T2 | start/finish instrument seam (ctx `startStep`/`finishStep` replace `emitStep`; per-kind detail + agent extractors; onBlocked forwards sessionId) + manager clock/stepId (`#stepSeq`/`#stepStart`, durationMs) (+15) | -| ba78590 | T3 | manager broadcasts `step-finished`/`run-started`/terminal `failed` (error/stdout/code) + `AgentStepError.detail` surfaced + blocked `sessionId` emit (+5) | -| 8ca31fb | T4 | `workflow-run-view` fold rebuild — `StepNode[]`, `stepStatus`, `groupRollup` (failed>blocked>running>done>pending), run-started/input/startedAt/blockedSessionId/error seeds; transitional `timeline` kept (+19, replaced 12 WF5 fold tests) | -| a294d5b | T5 | `relative-time` pure helper extracted from TopBar (+4) | -| 56022ff | T7 | Icon glyphs (`workflow-nodes`/`play`/`help-circle`/`x-circle`/`stop-square`) + TopBar Workflows segment uses `workflow-nodes` | -| 899ddc9 | T6 | `use-workflow-runs` consumes `workflow:run-started`, retires `pendingWf` hack | -| 01461e9 | T8 | hifi `RunDetail` — node timeline+glyphs+connectors, kind tags, durations, group rollup, step + agent detail boxes, header+relative-time, INPUTS strip, hifi respond panel+session note, failed footer; **dropped transitional `timeline`** from RunView+fold (−1 timeline-only fold test) | -| a7bf88f | T9 | hifi `WorkflowsView` rail — DEFINITIONS cards + RECENT RUNS + relative time + pipeline glyph empty state | -| 070da74 | T10 | hifi `WorkflowTriggerDialog` (kicker, tile, mono fields, required `*`, play-triangle Run) | -| c38f996 | T11 | `implement-ticket` fixture wraps worktree.create+agent in one `ctx.step` group (WHF-14 live gate); `notify(JSON)` result line preserved | +| c846eb0 | plan | spec (WPC-01..24) + design + tasks + AD-013 | +| 7732a89 | T1 | `repo-config.ts` — repo-local `postCreateCommand` reader (+10) | +| 4859446 | T2 | `post-create-hook.ts` — `runPostCreateHook` env/tail/timeout mapping + shared types (+12) | +| bce57f4 | T3 | `withPostCreateHook` run-iff-created decorator (+10) | +| dd3eeab | T4 | `index.ts` spawn seam + single wiring point | +| 0ce6177 | T5 | `HookFailureNotice` component + CSS | +| cd95f5a | T6 | both create dialogs surface hook failure | +| 0291a70 | F1 | **Verifier blocker** — shell settled on `close` only; extracted to `hook-shell.ts`, settles on `close` OR `exit`+grace (+7) | +| adff2bb | F2/F3 | pinned the 4000 literal (surviving mutant); real-git end-to-end for WPC-03's on-disk half (+3) | +| 98034eb | F4 | backdrop dismissal skipped the tree refresh | +| 663e2d3 | F5 | grace timer un-`unref`'d (paths were not independent); real-seam stderr + large-burst tests; shortened lingering pings (+2) | -**SPEC_DEVIATION (benign, Verifier-confirmed):** `RunDetail.css:14` + `WorkflowsView.css:21` -materialise `@keyframes pulse` component-locally — the handoff/design assumed a shared `pulse` -keyframe but `global.css` only had `fadeIn`/`popIn`/`toastIn`. No new named animation beyond the -handoff's set, no new tokens. Distilled as lesson **L-002** (candidate, `spec_deviation`): grep -`global.css` to confirm a referenced CSS keyframe exists before a UI design cites it as existing. +**Verifier (independent, author ≠ verifier) — round 1 FAIL → round 2 PASS, 4/4 findings +closed.** Round 1 caught a genuine blocker: resolving on `close` waits for stdio EOF, and +`spawn`'s timeout kills only `cmd.exe`, so a surviving grandchild held the pipes — measured +`exit` 1665 ms vs `close` 12969 ms, and 21000 ms with a detached grandchild. A hung script +would never settle: `worktrees:create` never resolved, dialog stuck on `busy`. Round 2 +re-probed the fixed seam with real processes: `pause` +152 ms, infinite loop +94 ms, +pipe-holding child **+119 ms**, detached grandchild **+467 ms**; output verified complete to a +1 MB single burst. Report: `.specs/features/worktree-post-create-hook/validation.md`. -**DONE (this session):** owner-run two-example UI gate PASSED → PR #67 created → admin-merged to -`main` (`668b2d4`) → issue #56 CLOSED → branch deleted. **No open next step for the Workflows epic.** +**Accepted mutation survivors (reasoned, not oversights):** `HOOK_FLUSH_GRACE_MS 250→0` and +removing the `close` handler are **equivalent mutants** — queued `data` events drain before the +timer callback either way, so the only observable difference is latency, and asserting +sub-250 ms latency on this contended box would be flaky. Verified empirically both ways. -**Deferred (spec Out of Scope):** Re-run action; live token-by-token agent stdout tail; persisted-run -read channel (`workflows:get`); backend `ctx.step` rollup status (renderer-derived); agent kind tag -shows `agent` (no `agentId` on the stream — box uses the step label as name). +**NEXT STEP (nothing else outstanding in code):** owner **visual/UAT pass** — WPC-12..16 are +marked `Built †` in the spec, not Verified: the renderer has no unit tests by convention and +the dialogs have never been rendered. Then push + PR with `Closes #` once the feature +issue exists (the repo's issue = feature = PR pipeline). **Live gate to run:** create a worktree +for `m: riade\source\Code` with `.app\config.json` → `{"postCreateCommand": ".\SetupSkills.cmd"}` +and confirm `.claude\skills` + `.codex\skills` junctions appear; then the same via a workflow. -**Baseline note:** `feature/workflows-ui` now at **486 tests / 36 files** green (was 440 at WF5 -Verifier; T1–T5 added ~+47 unit tests, T6–T11 added 0 per renderer/fixture hand-verify convention; -T4 replaced WF5's 12 fold tests, T8 dropped 1 timeline-only fold test). Gate: -`npm run typecheck && npm run lint && npm test` (+ `npm run build`). AD-005 `tree.test.ts` real-git -Windows flake did not fire in the Verifier's full run; re-run in isolation if a future gate flakes on it. +**Deferred (spec Out of Scope):** WPC-17..19 — the workflow run-timeline hook detail box (needs a +new `StepDetail` variant + a `RunDetail` branch); `result.hook` is already reachable by an author. +Also: multiple/ordered commands, other lifecycle hooks, Settings UI, trust prompt/allowlist, +process-tree kill, in-app re-run. -**Prior context:** `worktree-existing-branch` (PR #62), `topbar-version-indicator` (PR #63), -WF1–WF4 all merged. Pre-existing quirk: `src/main/ado-gateway.ts` is UTF-16 (git treats it as binary). -Open follow-ups: 3 transitive dev advisories (esbuild/form-data/undici); App.tsx -`useTasks`/`useConfig` extraction deferred (AD-004). +**Two environment findings (NOT code issues), worth acting on separately:** +1. **`npm test` is unreliable on this machine.** Real-git tests in `tree.test.ts` / + `worktree-manager.test.ts` intermittently exceed their **5000 ms default** timeout under load + (observed 5.1 / 6.1 / 8.4 / 44 s), then cascade to `EPERM` in `afterEach` because the + timed-out git child still holds the temp dir. The failing subset differs per run and both + files pass 71/71 in isolation. `--maxWorkers=2` is reliable AND faster (81–125 s vs 300 s) — + vitest oversubscribes this box. **Recommend a `testTimeout` bump and/or `maxWorkers` in + `vitest.config.ts`** (deliberately not changed here — out of feature scope). +2. **`NoDefaultCurrentDirectoryInExePath=1`** in the agent session env makes a bare + `SetupSkills.cmd` fail with code 1; it is not a persistent User/Machine variable. Harness + artifact, not a product bug — but it's why the README example uses `.\SetupSkills.cmd`. + (Same variable already noted for node-gyp builds.) + +**Prior context:** Workflows epic (#56) DONE + CLOSED; WF1–WF5 + hifi merged (PR #67). Baseline +before this feature: 489 tests / 36 files on `main`. Pre-existing quirk: +`src/main/ado-gateway.ts` is UTF-16 (git treats it as binary). Open follow-ups: 3 transitive dev +advisories (esbuild/form-data/undici); App.tsx `useTasks`/`useConfig` extraction deferred +(AD-004). diff --git a/.specs/features/worktree-post-create-hook/spec.md b/.specs/features/worktree-post-create-hook/spec.md index 08a3cc9..82fea2c 100644 --- a/.specs/features/worktree-post-create-hook/spec.md +++ b/.specs/features/worktree-post-create-hook/spec.md @@ -2,7 +2,7 @@ ## Problem Statement -A fresh `git worktree add` gives you a checkout, but not a *working* checkout. Repos +A fresh `git worktree add` gives you a checkout, but not a _working_ checkout. Repos increasingly need a one-shot local initialization step after the files land — `m:\triade\source\Code` ships `SetupSkills.cmd` (→ `SetupSkills.ps1`) whose only job is to create the `.claude\skills` and `.codex\skills` junctions that let multiple coding agents @@ -23,16 +23,16 @@ never get it at all — which is precisely the case that needs it most. Explicitly excluded. Documented to prevent scope creep. -| Feature | Reason | -| ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Multiple commands / ordered hook list | One command is enough — a repo that needs several chains them in its own `.cmd`/`.ps1`, exactly as `SetupSkills.cmd` already calls `SetupSkills.ps1`. | -| Other lifecycle hooks (pre-create, post-remove, post-checkout) | Only the post-create pain is real today. Adding a hook registry now would design for hooks nobody has asked for. | -| Settings-dialog UI for the command | The command is repo-local and checked in (AD decision below); there is nothing per-machine to edit. A global override can be layered later if needed. | -| Trust prompt / command allowlist | Deliberate: see the security assumption below. The set of repos in a registered workspace is already fully trusted by the app (it runs `git` in them). | -| Process-tree kill on timeout | The timeout kills the spawned shell; a detached grandchild may survive. A real tree-kill (`taskkill /T /F`) is a separate, Windows-specific concern. | -| Streaming live output into a terminal session | Considered and rejected during Specify — needs session lifecycle plumbing and severs the create's knowledge of whether init succeeded. | -| Re-run / retry action for a failed hook | The command is idempotent by convention (the reference script is) and re-runnable by hand from the worktree. An in-app re-run button is a v2 nicety. | -| Reading the hook command from the **new worktree's** copy | The source repo's `.app/config.json` is authoritative — reading the worktree's own copy would let a branch under development change what runs on checkout. | +| Feature | Reason | +| -------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Multiple commands / ordered hook list | One command is enough — a repo that needs several chains them in its own `.cmd`/`.ps1`, exactly as `SetupSkills.cmd` already calls `SetupSkills.ps1`. | +| Other lifecycle hooks (pre-create, post-remove, post-checkout) | Only the post-create pain is real today. Adding a hook registry now would design for hooks nobody has asked for. | +| Settings-dialog UI for the command | The command is repo-local and checked in (AD decision below); there is nothing per-machine to edit. A global override can be layered later if needed. | +| Trust prompt / command allowlist | Deliberate: see the security assumption below. The set of repos in a registered workspace is already fully trusted by the app (it runs `git` in them). | +| Process-tree kill on timeout | The timeout kills the spawned shell; a detached grandchild may survive. A real tree-kill (`taskkill /T /F`) is a separate, Windows-specific concern. **Note:** a surviving grandchild no longer delays the result — the shell settles on `exit` + a flush grace period, not on `close` (see `hook-shell.ts`). | +| Streaming live output into a terminal session | Considered and rejected during Specify — needs session lifecycle plumbing and severs the create's knowledge of whether init succeeded. | +| Re-run / retry action for a failed hook | The command is idempotent by convention (the reference script is) and re-runnable by hand from the worktree. An in-app re-run button is a v2 nicety. | +| Reading the hook command from the **new worktree's** copy | The source repo's `.app/config.json` is authoritative — reading the worktree's own copy would let a branch under development change what runs on checkout. | --- @@ -40,18 +40,18 @@ Explicitly excluded. Documented to prevent scope creep. Every ambiguity is resolved or recorded here — nothing is left silently unclear. -| Assumption / decision | Chosen default | Rationale | Confirmed? | -| -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | -| Where the command is declared | New repo-local `\.app\config.json`, key `postCreateCommand` | Mirrors the existing `\.app\config.json` reader (`workspace-config.ts`) one level down: read-on-use, no caching, malformed → silent fallback. The repo self-describes its own init and it travels with the repo. | **y** | -| Failure semantics | Keep the worktree, return `ok: true` with a `hook` failure payload | `git worktree add` already succeeded — the checkout is valid git. Destroying it (plus any base refresh / branch recut) over a fixable script error is the worse outcome. | **y** | -| Which create paths run it | All three — the hook lives inside `createWorktree()` | One seam, one behavior. Workflow-created worktrees are the case that most needs init (agents need the skills junctions). | **y** | -| Feedback surface | Inline in the dialog on failure; silent on success | No new noise on the happy path; the failure lands in the slot the create errors already use, next to an explicit "the worktree *was* created" note. | **y** | -| **Security: the command comes from repo content** | No confirmation prompt, no allowlist — it just runs | Owner accepted this trade-off when choosing the repo-local home. A registered workspace's repos are already trusted (the app runs `git` in them, and worktree paths are derived from them). Flagged here so it is a recorded decision, not an oversight: **cloning an untrusted repo into a registered workspace means its `postCreateCommand` runs on your next worktree create for that repo.** | **y** | -| Hosting shell | `spawn(cmd, { shell: true, windowsHide: true, cwd: })` — same shape as the existing `runShell` (`index.ts:70`) | `.cmd` files require a shell. Reusing the proven `ctx.sh` runner shape means one spawn idiom in the codebase, and `{code, stdout, stderr}` capture that never throws. | y (agent) | -| Timeout | Fixed **120000 ms**, not configurable | Long enough for a junction/copy/restore script, short enough that a hung hook doesn't wedge the create forever. A knob can be added when a real script needs one. | y (agent) | -| Captured-output bound | Combined stdout+stderr, **last 4000 characters** retained | An unbounded string from a chatty script would be held in the main process and shipped over IPC to the dialog. The tail is what diagnoses a failure. | y (agent) | -| Context handed to the command | Inherited `process.env` plus `PLAYGROUND_WORKTREE_PATH`, `PLAYGROUND_REPO_PATH`, `PLAYGROUND_BRANCH` | The reference script needs none of these (it resolves from `$PSScriptRoot`), but a generic script needs the branch/source-repo without re-deriving them. Cheap and additive. | y (agent) | -| Workflow run-timeline detail box | **P2**, not MVP | It needs a new `StepDetail` variant *and* a `RunDetail` render branch. The hook result is already reachable by a workflow author via the returned `CreateWorktreeResult.hook` without it. | y (agent) | +| Assumption / decision | Chosen default | Rationale | Confirmed? | +| ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| Where the command is declared | New repo-local `\.app\config.json`, key `postCreateCommand` | Mirrors the existing `\.app\config.json` reader (`workspace-config.ts`) one level down: read-on-use, no caching, malformed → silent fallback. The repo self-describes its own init and it travels with the repo. | **y** | +| Failure semantics | Keep the worktree, return `ok: true` with a `hook` failure payload | `git worktree add` already succeeded — the checkout is valid git. Destroying it (plus any base refresh / branch recut) over a fixable script error is the worse outcome. | **y** | +| Which create paths run it | All three — the hook lives inside `createWorktree()` | One seam, one behavior. Workflow-created worktrees are the case that most needs init (agents need the skills junctions). | **y** | +| Feedback surface | Inline in the dialog on failure; silent on success | No new noise on the happy path; the failure lands in the slot the create errors already use, next to an explicit "the worktree _was_ created" note. | **y** | +| **Security: the command comes from repo content** | No confirmation prompt, no allowlist — it just runs | Owner accepted this trade-off when choosing the repo-local home. A registered workspace's repos are already trusted (the app runs `git` in them, and worktree paths are derived from them). Flagged here so it is a recorded decision, not an oversight: **cloning an untrusted repo into a registered workspace means its `postCreateCommand` runs on your next worktree create for that repo.** | **y** | +| Hosting shell | `spawn(cmd, { shell: true, windowsHide: true, cwd: })` — same shape as the existing `runShell` (`index.ts:70`) | `.cmd` files require a shell. Reusing the proven `ctx.sh` runner shape means one spawn idiom in the codebase, and `{code, stdout, stderr}` capture that never throws. | y (agent) | +| Timeout | Fixed **120000 ms**, not configurable | Long enough for a junction/copy/restore script, short enough that a hung hook doesn't wedge the create forever. A knob can be added when a real script needs one. | y (agent) | +| Captured-output bound | Combined stdout+stderr, **last 4000 characters** retained | An unbounded string from a chatty script would be held in the main process and shipped over IPC to the dialog. The tail is what diagnoses a failure. | y (agent) | +| Context handed to the command | Inherited `process.env` plus `PLAYGROUND_WORKTREE_PATH`, `PLAYGROUND_REPO_PATH`, `PLAYGROUND_BRANCH` | The reference script needs none of these (it resolves from `$PSScriptRoot`), but a generic script needs the branch/source-repo without re-deriving them. Cheap and additive. | y (agent) | +| Workflow run-timeline detail box | **P2**, not MVP | It needs a new `StepDetail` variant _and_ a `RunDetail` render branch. The hook result is already reachable by a workflow author via the returned `CreateWorktreeResult.hook` without it. | y (agent) | **Open questions:** none — all resolved or logged above. @@ -193,59 +193,71 @@ then a workflow author can already read `result.hook` from `ctx.worktree.create` Medium scope — dimensions obviously present for this domain are covered; the rest are explicitly N/A. -| Dimension | Resolution | -| -------------------------------- | ------------------------------------------------------------------------------------------------------- | -| Input validation & bounds | WPC-06 (blank / non-string / malformed), WPC-05 (time bound), WPC-11 (output bound) | -| Failure / partial-failure states | WPC-03, WPC-04, WPC-05 — every failure mode keeps the worktree and reports; WPC-08 defines "no worktree" | -| Observability | WPC-07 (`console.error` on malformed config), WPC-12/13 (surfaced to the user), WPC-17 (run timeline) | -| Concurrency / ordering | WPC-22 (independent), and the hook is strictly ordered after `git worktree add`, before the return | -| State-transition integrity | WPC-08 — hook runs **iff** a worktree was actually created | -| Idempotency / retry | No automatic retry; the hook runs at most once per successful create. Re-running is manual (Out of Scope) | -| Auth boundaries & rate limits | N/A because the command runs in-process on the user's own machine at their own request; no remote caller | +| Dimension | Resolution | +| -------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | +| Input validation & bounds | WPC-06 (blank / non-string / malformed), WPC-05 (time bound), WPC-11 (output bound) | +| Failure / partial-failure states | WPC-03, WPC-04, WPC-05 — every failure mode keeps the worktree and reports; WPC-08 defines "no worktree" | +| Observability | WPC-07 (`console.error` on malformed config), WPC-12/13 (surfaced to the user), WPC-17 (run timeline) | +| Concurrency / ordering | WPC-22 (independent), and the hook is strictly ordered after `git worktree add`, before the return | +| State-transition integrity | WPC-08 — hook runs **iff** a worktree was actually created | +| Idempotency / retry | No automatic retry; the hook runs at most once per successful create. Re-running is manual (Out of Scope) | +| Auth boundaries & rate limits | N/A because the command runs in-process on the user's own machine at their own request; no remote caller | | External-dependency failure | N/A because the hook makes no network or service call — the spawned process is the only dependency, covered by WPC-04/05 | -| Data lifecycle / expiry | N/A because the hook persists nothing — the captured output lives only in the returned result | +| Data lifecycle / expiry | N/A because the hook persists nothing — the captured output lives only in the returned result | --- ## Requirement Traceability -| Requirement ID | Story | Phase | Status | -| -------------- | ------------------------------ | ----- | ------- | -| WPC-01 | P1: Init command runs | Tasks | Pending | -| WPC-02 | P1: Init command runs | Tasks | Pending | -| WPC-03 | P1: Init command runs | Tasks | Pending | -| WPC-04 | P1: Init command runs | Tasks | Pending | -| WPC-05 | P1: Init command runs | Tasks | Pending | -| WPC-06 | P1: Init command runs | Tasks | Pending | -| WPC-07 | P1: Init command runs | Tasks | Pending | -| WPC-08 | P1: Init command runs | Tasks | Pending | -| WPC-09 | P1: Init command runs | Tasks | Pending | -| WPC-10 | P1: Init command runs | Tasks | Pending | -| WPC-11 | P1: Init command runs | Tasks | Pending | -| WPC-12 | P1: Hook failure is visible | Tasks | Pending | -| WPC-13 | P1: Hook failure is visible | Tasks | Pending | -| WPC-14 | P1: Hook failure is visible | Tasks | Pending | -| WPC-15 | P1: Hook failure is visible | Tasks | Pending | -| WPC-16 | P1: Hook failure is visible | Tasks | Pending | -| WPC-17 | P2: Run-timeline detail | - | Pending | -| WPC-18 | P2: Run-timeline detail | - | Pending | -| WPC-19 | P2: Run-timeline detail | - | Pending | -| WPC-20 | Edge case | Tasks | Pending | -| WPC-21 | Edge case | Tasks | Pending | -| WPC-22 | Edge case | Tasks | Pending | -| WPC-23 | Edge case | Tasks | Pending | -| WPC-24 | Edge case | Tasks | Pending | +**Status after execution:** the 21 P1 requirements are **Verified** — T1–T6 plus fix commits +F1–F4, independently validated by a fresh Verifier (round 2 PASS, all four round-1 findings +closed; see `validation.md`). WPC-17..19 stay **Pending** as the deliberately deferred P2 slice. + +| Requirement ID | Story | Phase | Status | +| -------------- | --------------------------- | ----- | -------- | +| WPC-01 | P1: Init command runs | Done | Verified | +| WPC-02 | P1: Init command runs | Done | Verified | +| WPC-03 | P1: Init command runs | Done | Verified | +| WPC-04 | P1: Init command runs | Done | Verified | +| WPC-05 | P1: Init command runs | Done | Verified | +| WPC-06 | P1: Init command runs | Done | Verified | +| WPC-07 | P1: Init command runs | Done | Verified | +| WPC-08 | P1: Init command runs | Done | Verified | +| WPC-09 | P1: Init command runs | Done | Verified | +| WPC-10 | P1: Init command runs | Done | Verified | +| WPC-11 | P1: Init command runs | Done | Verified | +| WPC-12 | P1: Hook failure is visible | Done | Built † | +| WPC-13 | P1: Hook failure is visible | Done | Built † | +| WPC-14 | P1: Hook failure is visible | Done | Built † | +| WPC-15 | P1: Hook failure is visible | Done | Built † | +| WPC-16 | P1: Hook failure is visible | Done | Built † | +| WPC-17 | P2: Run-timeline detail | - | Pending | +| WPC-18 | P2: Run-timeline detail | - | Pending | +| WPC-19 | P2: Run-timeline detail | - | Pending | +| WPC-20 | Edge case | Done | Verified | +| WPC-21 | Edge case | Done | Verified | +| WPC-22 | Edge case | Done | Verified | +| WPC-23 | Edge case | Done | Verified | +| WPC-24 | Edge case | Done | Verified | **ID format:** `WPC-[NUMBER]` **Coverage:** 24 total — 21 in the P1 MVP slice (WPC-01..16, WPC-20..24), 3 deferred to P2 (WPC-17..19). +† **Built, not visually confirmed.** WPC-12..16 are renderer ACs: implemented, typechecked and +built, with the data path verified by inspection — but the renderer carries no unit tests by +project convention (`TESTING.md`), and **no visual/UAT pass has been run**. They become +_Verified_ only after the owner's visual gate. The same applies to the spec's Success Criteria, +which remain unconfirmed on real hardware. + **Unit-testable vs hand-verified** (per `.specs/codebase/TESTING.md`): WPC-01..11 and -WPC-20..24 are main-process logic → **unit tests** (real-temp-dir + injected-fake runner, no -real spawn in the assertions of the decision logic). WPC-12..16 and WPC-19 are renderer → -**hand-verified** by convention. The actual `spawn(shell:true)` seam in `index.ts` is a thin -OS shell → hand-verified. +WPC-20..24 are main-process logic → **unit tests** (real-temp-dir + injected-fake runner for the +decision logic). The shell seam is the one deviation from the original plan: it was to be a +hand-verified thin shell inside `index.ts`, but the Verifier proved its settle condition wrong +in a way no fake could catch, so it now lives in `src/main/hook-shell.ts` with **real-process +tests** (`hook-shell.test.ts`). WPC-12..16 are renderer → built + inspected, visual pass +outstanding (see †). --- diff --git a/.specs/features/worktree-post-create-hook/tasks.md b/.specs/features/worktree-post-create-hook/tasks.md index 835b377..0eefc13 100644 --- a/.specs/features/worktree-post-create-hook/tasks.md +++ b/.specs/features/worktree-post-create-hook/tasks.md @@ -12,9 +12,36 @@ review, Verifier, discrimination sensor). --- **Design**: `.specs/features/worktree-post-create-hook/design.md` -**Status**: Approved +**Status**: Done (T1–T6 committed + 3 Verifier fix commits; validation round 2 in progress) **Branch**: `feature/worktree-post-create-hook` **Baseline**: 489 tests / 36 files (green on `main`, verified) +**Final**: 531 tests / 39 files green + +## Execution Record + +| Task | Commit | Tests added | Notes | +| ---- | ------ | ----------- | ----- | +| T1 | `7732a89` | +10 (planned +9) | non-string got its own test | +| T2 | `4859446` | +12 | as planned | +| T3 | `bce57f4` | +10 | as planned | +| T4 | `dd3eeab` | 0 | thin shell, build gate | +| T5 | `0ce6177` | 0 | renderer, build gate | +| T6 | `cd95f5a` | 0 | renderer, build gate; `build:win` OK | +| F1 | `0291a70` | +7 | **Verifier blocker** — hook shell settled on `close` only; extracted to `hook-shell.ts` with real-process tests | +| F2+F3 | `adff2bb` | +3 | pinned the 4000 literal (surviving mutant); real-git end-to-end for WPC-03's on-disk half | +| F4 | `98034eb` | 0 | backdrop dismissal skipped the tree refresh | + +**Deviation from the design:** `runHookShell` was planned to live in `index.ts` as a +hand-verified thin shell. The Verifier proved its settle condition was wrong in a way no fake +could catch, so it was extracted to `src/main/hook-shell.ts` and given real-process tests. The +design's module table is otherwise unchanged. + +**Environment note (not a code issue):** `npm test` with default worker count is unreliable on +this machine — real-git tests in `tree.test.ts` / `worktree-manager.test.ts` intermittently +exceed their 5000 ms default timeout under load (observed 5.1 s / 6.1 s / 44 s), with a +cascading `EPERM` in `afterEach` because the timed-out git child still holds the temp dir. +`npx vitest run --maxWorkers=2` runs the identical suite reliably and faster. Surfaced as a +repo follow-up; deliberately not changed here (out of feature scope). --- diff --git a/.specs/features/worktree-post-create-hook/validation.md b/.specs/features/worktree-post-create-hook/validation.md new file mode 100644 index 0000000..6afe1ef --- /dev/null +++ b/.specs/features/worktree-post-create-hook/validation.md @@ -0,0 +1,638 @@ +# Worktree Post-Create Hook Validation + +**Date**: 2026-07-29 +**Spec**: `.specs/features/worktree-post-create-hook/spec.md` +**Diff range**: round 1 `c846eb0..cd95f5a`; **round 2 `cd95f5a..98034eb`** (full range `c846eb0..98034eb`) on `feature/worktree-post-create-hook` +**Verifier**: independent sub-agent (author ≠ verifier), read-only over the implementation +**Scope**: P1 slice only — WPC-01..16, WPC-20..24. WPC-17..19 are deferred P2 and are **not** treated as gaps. + +> **CURRENT VERDICT (round 2, 2026-07-29): ✅ PASS.** The blocker is genuinely closed — +> independently re-probed against the new `hook-shell.ts`, every hung-command case now settles +> within 94–467 ms of the deadline (was 12969 ms / 21000 ms) and still reports +> `{code:-1, timedOut:true}`, with **no output loss** on the normal path (verified to 1 MB). +> All four round-1 findings are Closed. Two new **non-blocking** issues were found: the suite's +> pre-existing real-git flakiness got measurably worse, and three mutants survive on the new +> seam. See **[Round 2](#round-2-re-verification-2026-07-29)** at the end for the full evidence. +> The round-1 analysis below is retained verbatim as the historical record. + +**Round-1 verdict (superseded): ⚠️ PASS WITH GAPS** — the decision logic is comprehensively covered and highly +discriminating (20/21 mutants killed), every gate is green at the expected baseline, and two of +the three author claims hold. One claim does **not** hold: the `runHookShell` timeout seam cannot +deliver WPC-05's contract for the exact case the timeout exists for. That is a real +implementation defect, found by direct empirical probe, and it is the one blocking item. + +--- + +## Task Completion + +| Commit | Deliverable | Status | Notes | +| --------- | -------------------------------------------------- | ------- | ----- | +| `7732a89` | `repo-config.ts` + 10 unit tests | ✅ Done | - | +| `4859446` | `post-create-hook.ts` runner + shared types + 12 tests | ✅ Done | - | +| `bce57f4` | `withPostCreateHook` decorator + 10 tests | ✅ Done | - | +| `dd3eeab` | `runHookShell` spawn seam + single wiring point | ⚠️ Partial | Wiring correct; the timeout path does not return within the bound — see Gap 1 | +| `0ce6177` | `HookFailureNotice.tsx` + `.css` | ✅ Done | Renderer, convention-exempt from unit tests | +| `cd95f5a` | Both dialogs hold `hookFailure` state | ✅ Done | Renderer, convention-exempt from unit tests | + +Files claimed in scope, files actually touched — exact match, no scope creep: +`src/main/{repo-config,post-create-hook}.ts` + `.test.ts`, `src/main/index.ts`, +`src/shared/worktrees.ts`, `src/renderer/src/components/{HookFailureNotice.tsx,HookFailureNotice.css,NewWorktreeDialog.tsx,StartWorkDialog.tsx}`. +`worktree-manager.ts`, `workflow-ctx.ts`, `workspace-config.ts` and all of their tests are +**untouched** (verified: `git diff --name-only c846eb0..cd95f5a` returns 0 rows for them). + +--- + +## Spec-Anchored Acceptance Criteria + +Test files: `PCH` = `src/main/post-create-hook.test.ts`, `RC` = `src/main/repo-config.test.ts`. + +| Criterion | Spec-defined outcome | `file:line` + assertion | Result | +| --------- | -------------------- | ----------------------- | ------ | +| **WPC-01** command runs through a shell with `cwd` = new worktree, before returning | shell invoked once, `cwd` === created worktree path | `PCH:173` — `expect(calls[0].cwd).toBe('M:\\src\\Code-feature-x')`; `PCH:116` — `expect(calls[0].cmd).toBe('SetupSkills.cmd')`; shell-hosting by inspection `index.ts:101` — `shell: true` | ✅ PASS | +| **WPC-02** exit 0 ⇒ `{ok:true, path, hook:{ok:true, command, code:0, output}}` | whole payload, field by field | `PCH:174-181` — `expect(result.ok).toBe(true)`, `expect(result.path).toBe('M:\\src\\Code-feature-x')`, `expect(result.hook).toEqual({ok:true, command:'SetupSkills.cmd', code:0, output:'junctions created'})` | ✅ PASS | +| **WPC-03** exit N ⇒ `{ok:true, path, hook:{ok:false, code:N, …}}` (payload half) | `ok` stays true, `path` preserved, `hook.ok` false, `hook.code` = N | `PCH:193-197` — `expect(result.ok).toBe(true)`, `expect(result.path).toBe('M:\\src\\Code-feature-x')`, `expect(result.error).toBeUndefined()`, `expect(result.hook?.ok).toBe(false)`, `expect(result.hook?.code).toBe(1)` | ✅ PASS | +| **WPC-03** worktree still exists on disk, no rollback (filesystem half) | directory present after a failed hook | **no test asserts on-disk state.** Structural evidence only: no removal call in the diff; `worktree-manager.test.ts:271` — `expect(existsSync(result.path!)).toBe(true)` proves `ok:true+path ⇒ dir exists` pre-feature | ⚠️ No direct evidence — structurally sound (see Claim 1) | +| **WPC-04** spawn failure ⇒ `hook:{ok:false, code:-1, …}` with error text in `output` | `ok:false`, `code:-1`, error text present | `PCH:62-64` — `expect(hook.ok).toBe(false)`, `expect(hook.code).toBe(-1)`, `expect(hook.output).toContain('spawn EACCES')`; real seam includes it: `index.ts:110` — `stderr: stderr + String(err)` | ✅ PASS (mapping tested, seam by inspection) | +| **WPC-05** not exited in 120000 ms ⇒ kill + `hook:{ok:false, code:-1, timedOut:true, output:}` | kill happens; result returned with exactly those values | Mapping: `PCH:72-75` — `expect(hook.ok).toBe(false)`, `expect(hook.code).toBe(-1)`, `expect(hook.timedOut).toBe(true)`, `expect(hook.output).toBe('Repo raiz : ...')`. Budget: `PCH:101-102` — `expect(calls[0].timeoutMs).toBe(120000)` + `expect(HOOK_TIMEOUT_MS).toBe(120000)`. Kill: `index.ts:103-117`, hand-verified | ❌ **GAP** — mapping and budget correct, but the seam does not return within the bound (Claim 3 / Gap 1) | +| **WPC-06** absent / unreadable / malformed / missing / blank / non-string ⇒ no command, **no `hook` property** | `null` from the reader; result has no `hook` key | Reader: `RC:46,52,58,64,69,72,78,96` — all `expect(repoPostCreateCommand(...)).toBeNull()`. Wrapper: `PCH:276-278` — `expect(calls).toHaveLength(0)`, `expect('hook' in result).toBe(false)`, `expect(result).toEqual({ok:true, path:'M:\\src\\Code-feature-x'})` | ✅ PASS | +| **WPC-07** malformed JSON ⇒ logged via `console.error`, silent fallback | exactly one `console.error`, message names the file; returns null | `RC:78-80` — `expect(repoPostCreateCommand(dir)).toBeNull()`, `expect(logged).toHaveLength(1)`, `expect(String(logged[0][0])).toContain(join(dir,'.app','config.json'))` | ✅ PASS | +| **WPC-08** no worktree ⇒ no command; `reuse`/`recreate` successes ⇒ command runs | shell never called and no `hook` key on every no-worktree outcome; called on reuse/recreate | No-run: `PCH:209-211` (branch-exists) — `expect(calls).toHaveLength(0)`, `expect('hook' in result).toBe(false)`, `expect(result.conflict).toBe('branch-exists')`; `PCH:223-224` (create failed); `PCH:236-237` (ok but no path). Run: `PCH:249-250` — `expect(calls[0].cwd).toBe('M:\\src\\Code-feature-reuse')`, `expect(result.hook?.ok).toBe(true)`; `PCH:262-263` (recreate). Each enumerated outcome is proven `ok:false` upstream: `worktree-manager.test.ts:307` (base unknown), `:315` (empty template), `:295` (target exists), `:396` (branch live elsewhere), `:519,:529` (blocked refresh) | ✅ PASS | +| **WPC-09** env = inherited `process.env` + the three `PLAYGROUND_*` vars | each var equals the corresponding context value; inheritance preserved | `PCH:83-85` — `expect(calls[0].env.PLAYGROUND_WORKTREE_PATH).toBe('M:\\triade\\source\\Code-feature-x')`, `…PLAYGROUND_REPO_PATH).toBe('M:\\triade\\source\\Code')`, `…PLAYGROUND_BRANCH).toBe('feature/x')`; `PCH:93` — `expect(calls[0].env.PATH).toBe(process.env.PATH)` | ✅ PASS (all four killed as mutants M5/M5b/M5c/M5d) | +| **WPC-10** all three create paths run the hook; no caller can opt out | both consumers bound to the wrapper; no bare `createWorktree` call site | `index.ts:231` — `withPostCreateHook(createWorktree, {readCommand: repoPostCreateCommand, shell: runHookShell})`; `index.ts:238` — IPC handler calls `createWorktreeWithHook(...)`; `index.ts:348` — `create: createWorktreeWithHook`. Exhaustive grep: the only production reference to bare `createWorktree` in `src/` is the wrapper argument itself | ✅ PASS (structural — see Claim 2; no automated regression guard) | +| **WPC-11** output > 4000 chars ⇒ **last** 4000 | length exactly 4000, tail retained | `PCH:124-125` — `expect(hook.output).toHaveLength(HOOK_OUTPUT_MAX_CHARS)`, `expect(hook.output.endsWith('TAIL')).toBe(true)` | ⚠️ **Partial** — "last, not first" is pinned (mutant M3 killed); the **value 4000** is not: the assertion references the constant, so changing it to 2000 keeps the suite green (mutant M7 SURVIVED) | +| **WPC-12** New Worktree dialog stays open showing path + command + code/timeout + output | four elements rendered, dialog not closed | `NewWorktreeDialog.tsx:105-108` — `if (result.hook && !result.hook.ok) { setHookFailure({path: result.path, hook: result.hook}); setBusy(false); return }` (no `onCreated`/`onClose` ⇒ stays open); `HookFailureNotice.tsx:37` path, `:38` command, `:39-41` `hook.timedOut ? 'timed out and was stopped' : \`exit code ${hook.code}\``, `:42` `hook.output !== '' && 
{hook.output}
` | ✅ PASS (inspection; renderer is unit-test-exempt per TESTING.md — no automated evidence) | +| **WPC-13** Start Work dialog shows the same four elements | as WPC-12 | `StartWorkDialog.tsx:117-123` (identical branch) + same `HookFailureNotice` lines | ✅ PASS (inspection) | +| **WPC-14** an action dismisses and proceeds with tree refresh + select | the action calls the happy path's `onCreated(path)` | `NewWorktreeDialog.tsx:210` / `StartWorkDialog.tsx:241` — `onProceed={() => onCreated(hookFailure.path)}`; `HookFailureNotice.tsx:47-49` "Continue" button; `App.tsx:176-180` — `worktreeCreated` ⇒ `refreshAndSelect(worktreePath)` | ✅ PASS (inspection) | +| **WPC-15** `hook.ok === true` OR no `hook` ⇒ today's behavior exactly | the advisory branch is entered only on `hook && !hook.ok` | `NewWorktreeDialog.tsx:105` / `StartWorkDialog.tsx:120` — guard `result.hook && !result.hook.ok`, else `onCreated(result.path)` at `:110` / `:126` | ✅ PASS (inspection) | +| **WPC-16** create button cannot re-submit the same create | button not reachable while the notice is up | `NewWorktreeDialog.tsx:206-235` — the `hookFailure ?` branch replaces the whole `