diff --git a/.specs/LESSONS.md b/.specs/LESSONS.md index 2fa5b6a..1c773bd 100644 --- a/.specs/LESSONS.md +++ b/.specs/LESSONS.md @@ -24,6 +24,24 @@ Seen once or not yet corroborated. Tracked, not trusted. - evidence: RunDetail.css:14 / WorkflowsView.css:21 (SPEC_DEVIATION pulse keyframe) (src/renderer/**/*.css) - last seen: 2026-07-06T21:57:51Z +### L-003 — When wrapping a spawned process with a timeout, settle the promise on 'exit' plus a short flush grace period, never on 'close' alone: 'close' waits for stdio EOF and killing a shell does not kill its children, so a surviving grandchild holds the inherited pipes and the promise can lag by seconds or never settle +- signal: `ac_gap` · recurrence: 1 feature(s) · scope: `child-process` · harmful: 0 +- features: worktree-post-create-hook +- evidence: validation.md round-1 blocker; src/main/hook-shell.ts:71 (child-process) +- last seen: 2026-07-29T22:37:03Z + +### L-004 — Assert a spec-defined bound against its literal value, not against the constant that implements it: expect(x).toHaveLength(MAX_CHARS) is self-referential and survives a mutation of MAX_CHARS itself +- signal: `surviving_mutant` · recurrence: 1 feature(s) · scope: `testing` · harmful: 0 +- features: worktree-post-create-hook +- evidence: mutant R1/M7; post-create-hook.test.ts output-tail test (testing) +- last seen: 2026-07-29T22:37:04Z + +### L-005 — Before adding real-process or real-git tests, check whether existing suites already sit near the default per-test timeout: the extra parallel load alone can push them over it, turning a green gate red without any production change +- signal: `gate_fail` · recurrence: 1 feature(s) · scope: `testing` · harmful: 0 +- features: worktree-post-create-hook +- evidence: validation.md round-2 gate section; tree.test.ts / worktree-manager.test.ts timeouts (testing) +- last seen: 2026-07-29T22:37:06Z + ## Quarantined (failed when applied — ignore) A confirmed lesson that recurred alongside failure. Kept for the maintainer to review. diff --git a/.specs/STATE.md b/.specs/STATE.md index 0607a92..f04ce45 100644 --- a/.specs/STATE.md +++ b/.specs/STATE.md @@ -19,61 +19,78 @@ 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 -**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/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..82fea2c --- /dev/null +++ b/.specs/features/worktree-post-create-hook/spec.md @@ -0,0 +1,273 @@ +# 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. **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. | + +--- + +## 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 + +**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 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 †). + +--- + +## 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..0eefc13 --- /dev/null +++ b/.specs/features/worktree-post-create-hook/tasks.md @@ -0,0 +1,377 @@ +# 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**: 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). + +--- + +## 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. 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 `