Refactor canvas workbench seams and image generation capabilities

.github/workflows/ci.yml

@@ -0,0 +1,29 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  verify:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Setup pnpm
+        uses: pnpm/action-setup@v4
+        with:
+          version: 9
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 20
+          cache: pnpm
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      - name: Verify
+        run: pnpm verify

AGENTS.md

@@ -1,43 +1,64 @@
 # Agent Guidelines
 
-## Commit & Pull Request Guidelines
-
-Follow Conventional Commit style: `feat(scope): ...`, `fix(scope): ...`, `refactor(scope): ...`, `perf: ...`. Keep scopes specific (e.g., `renderer`, `library`, `editor`, `canvas`, `chat`, `ai`, `router`). For PRs, include:
-
-- clear summary and rationale
-- linked issue/task
-- test evidence (`pnpm test`, `pnpm lint`, `pnpm build`)
-- screenshots or short recordings for UI changes
-
 ## General
 
 - Do not tell me I am right all the time. Be critical. We are equals. Stay neutral and objective.
 - Never be sycophantic. If you disagree, say so directly. If my suggestion would make the code worse, push back.
 - Do not excessively use emojis.
 - Always read and understand code before proposing changes. Never suggest modifications to code you have not inspected.
+- When I ask for a change overview, explain the post-change overall architecture first. Cover every major affected file, its role in that architecture, and for any large, high-responsibility, or newly created component or module, include a concise description of its internal logic.
 - Do what has been asked; nothing more, nothing less. Do not over-engineer.
 - For small decisions (naming, local refactors, implementation details), decide on your own and move on. Only ask when the choice is irreversible or affects security/architecture.
 - When refactoring or creating new module, first propose what you consider best practice and let the user decide, rather than immediately compromising the workspace code
+- When modifying a module, search `docs/tasks` for related unfinished notes first; do not ignore existing task context and duplicate or conflict with in-flight work.
+- When updating this file, keep rules short and dense: say when to do something and what anti-pattern to avoid, without extra narration.
+
+## Code Convention
+
+- Split logic or components when a unit has more than one real responsibility, repeated behavior, or state/side-effect flow that makes ownership unclear; do not split for tiny one-off paths, prop-forwarding wrappers, or abstractions that only make the file shorter while keeping the same coupling.
+- If a module keeps getting patched and review keeps finding bugs in the same area, stop and decide whether a full refactor is safer than another local fix.
 - Prefer shared helpers in `src/utils/<function>.ts`, with re-exports from `src/utils/index.ts`.
-- Do not inline runtime ID generation with `randomUUID`, `Date.now`, or `Math.random`; reuse the shared ID helper instead.
-- For canvas insert, duplicate, delete, and upsert flows, reuse the shared collision and selection handling instead of re-implementing ad hoc logic at new call sites.
+
+## Long Tasks
+
+- Treat a task as long when it is too large or coupled to finish safely in one session.
+- Use the first session for orchestration only: split the work into small slices with clear validation boundaries.
+- Persist progress to files, not chat history. Keep:
+  - a markdown task note for scope, architecture decisions, risks, validation, and handoff
+  - a minimal JSON task list for execution state only
+- Name long-task markdown and JSON files consistently by module/topic so they pair cleanly across sessions and store in docs/tasks; keep markdown for session context and JSON for execution state, not mixed duplicates.
+- Keep the JSON terse: stable task statuses such as `pending`, `in_progress`, `blocked`, `done`, `rolled_back`; `passes` as the completion gate; baseline/current task; rollback notes only when not obvious.
+- If a slice fails validation and is not fixed immediately, mark it `blocked` or `rolled_back`, record the first actionable failure in the markdown note, and stop claiming progress.
+
+## Compact Instructions
+
+- When compressing context or handoff material, preserve information in this order and drop lower-priority material first.
+1. Architecture decisions. Do not summarize away the decision, rationale, boundary, or chosen tradeoff.
+2. Modified files and critical changes. Keep an explicit file list and the key change in each file.
+3. Validation state. Record pass or fail per relevant command.
+4. Unresolved TODOs and rollback notes. Keep them explicit.
+5. Tool output. Reduce it to pass or fail plus the first actionable error unless the full raw output is needed for debugging.
 
 ## Testing
 
 - Keep implementation work and test-writing work logically separate. If both are needed, finish implementation first and write tests as a separate step.
-- Do not use subagents for implementation or test authoring. Subagents are reserved for review and explore only.
+- Prefer keeping implementation and test authoring in the main agent. Use subagents there only when the work has already been decomposed into explicit, low-coupling slices with clear external task state.
 - Only write unit tests for pure functions.
 - Treat pure functions strictly: deterministic input/output logic with no I/O, shared mutable state, framework lifecycle, network, storage, timer, or rendering side effects.
 - Do not add unit tests for components, hooks, stores, routes, integration flows, or any side-effectful or non-pure module.
 - Do not expand existing nonconforming tests. They may be deleted when touched or when cleanup is requested.
+- For browser-based validation of local UI flows, use `agent-browser` for interactive smoke tests and lightweight end-to-end functional checks. Do not use it as the default tool for extracting or reading page content.
+- Only codify an `agent-browser` flow under `scripts/` and `package.json` when it is stable and expected to be rerun regularly; otherwise a one-off manual smoke pass is enough.
 
 ## Subagents
 
-- Use subagents only for review and explore tasks.
-- Do not use subagents for implementation, refactoring, test authoring, orchestration, or integration work.
+- Prefer using subagents for bounded review and exploration.
+- Prefer keeping orchestration, implementation, refactoring, integration, and test authoring in the main agent unless long-task slices and external task state are already explicit.
 - Explore subagents should answer bounded codebase questions or gather context only.
 - Review subagents may be used for architecture, performance, and bug or missing-functionality review passes.
-- If a task is too large or too coupled to split safely, do not delegate delivery work to subagents; ask the user to narrow scope or help decompose it instead.
+- Default to the minimum review surface that matches the change. For complex logic changes, usually start with architecture plus bug/regression; add performance only when hot paths or render/update frequency changed.
+- If a task is too large or too coupled to split safely, prefer decomposing it first or narrowing scope instead of delegating delivery work immediately.
+- In review prompts, state any accepted current behaviors and out-of-scope interactions explicitly so subagents do not keep re-reporting them.
 
 ## Code Review
 
@@ -46,6 +67,8 @@ Follow Conventional Commit style: `feat(scope): ...`, `fix(scope): ...`, `refact
 - After implementation and any relevant tests pass, the main agent must decide how many review subagents to run, and which types to run, based on the scope of the current changes and the results of the previous review round.
 - Dispatch architecture, performance, and bug or missing-functionality review subagents only when that area could be affected by the current changes, or when a previous review in that area found issues that still need revalidation.
 - If a review area is clearly unaffected by the current changes and the last pass for that area found no issues, do not rerun that subagent.
+- If repeated review findings are symptoms of the same root cause, consolidate them and treat them as one problem to fix, not as an excuse to keep cycling shallow review passes.
+- If review keeps surfacing new issues in the same file or state transition logic, prefer one deeper holistic re-review after the structural fix instead of many narrow reruns.
 - Do not commit an independent module or step until the review subagent passes selected for that step have finished and their findings have been resolved or explicitly accepted.
 - The main agent remains responsible for dispatching those review passes, consolidating findings, resolving conflicts, and deciding the final changes.
 
@@ -55,3 +78,12 @@ Follow Conventional Commit style: `feat(scope): ...`, `fix(scope): ...`, `refact
 - Use the gh tool for GitHub-related operations.
 - Atomic development: when executing a multi-step plan, commit after each independent step completes only after relevant tests pass and the required review subagent passes are complete. Do not accumulate all
   changes into one final commit.
+
+## Commit & Pull Request Guidelines
+
+Follow Conventional Commit style: `feat(scope): ...`, `fix(scope): ...`, `refactor(scope): ...`, `perf: ...`. Keep scopes specific (e.g., `renderer`, `library`, `editor`, `canvas`, `chat`, `ai`, `router`). For PRs, include:
+
+- clear summary and rationale
+- linked issue/task
+- test evidence (`pnpm test`, `pnpm lint`, `pnpm build`)
+- screenshots or short recordings for UI changes

docs/tasks/canvas-active-workbench-boundary.json

@@ -0,0 +1,26 @@
+{
+  "baseline": "a3a4cc3",
+  "currentTaskId": null,
+  "tasks": [
+    {
+      "id": "1",
+      "title": "introduce active-workbench selectors and seam",
+      "status": "done",
+      "passes": true
+    },
+    {
+      "id": "2",
+      "title": "migrate first-wave consumers and remove implicit active store apis",
+      "status": "done",
+      "passes": true,
+      "rollback": "revert the active-workbench boundary slice commit"
+    },
+    {
+      "id": "3",
+      "title": "run validation and record follow-up risks",
+      "status": "done",
+      "passes": true,
+      "rollback": "revert validation follow-up changes"
+    }
+  ]
+}

docs/tasks/canvas-active-workbench-boundary.md

@@ -0,0 +1,53 @@
+# Canvas Active Workbench Boundary
+
+- Baseline commit: `a3a4cc3`
+- Branch: `feat/canvas-optimize`
+- Scope: shrink the canvas active-workbench application boundary by removing implicit active-store mutation APIs, introducing pure selectors, and routing first-wave consumers through a dedicated active-workbench seam
+
+## Decisions
+
+- Do not touch export domain semantics or unify the stage snapshot fallback in this slice.
+- Keep `useCanvasStore` as the global state container for collection, lifecycle, and UI state.
+- Remove implicit active-workbench mutation/history convenience methods from `CanvasState`; keep only explicit `...InWorkbench(workbenchId, ...)` store APIs.
+- Introduce one `useActiveCanvasWorkbench` seam that binds the current `activeWorkbenchId` to a stable read/write contract with null-safe no-op behavior.
+- Tighten the seam contract further after review: if `activeWorkbenchId` is set but the referenced workbench no longer exists, the seam must still collapse to the null/false/no-op contract instead of exposing a stale id-bound facade.
+- Keep narrow consumers on pure selectors plus explicit `...InWorkbench` store APIs when the broad seam would cause unnecessary render churn; `useCanvasHistory` is the first carve-out.
+- Introduce pure canvas store selectors under `src/features/canvas/store` so active workbench lookup, slice lookup, root count, and undo/redo availability stop being reimplemented across consumers.
+- Limit consumer migration in this slice to `CanvasViewport`, `useCanvasInteraction`, `useCanvasLayers`, `useCanvasEngine`, `useCanvasHistory`, `useCanvasPropertiesPanelModel`, `useCanvasWorkbenchActions`, and `useCanvasExport`.
+- Keep the recovery planner unchanged, but patch `useCanvasPageModel` so post-navigation recovery explicitly re-aligns `activeWorkbenchId` after fallback/create navigation. This fixes the observed delete-current recovery hole without changing recovery policy.
+- Guard post-navigation recovery completion with a token plus committed-route check so an older recovery promise cannot overwrite a newer route selection.
+- Back the pending-recovery marker with state as well as refs so clearing or superseding a recovery attempt still triggers a fresh recovery-plan evaluation.
+
+## Risks
+
+- `useCanvasStore` remains broadly readable for pure UI state and some non-migrated hooks; this slice narrows active-workbench use cases, not all store access.
+- Export still retains the existing stage snapshot preview fallback by design.
+- `useActiveCanvasWorkbench` still serves broad consumers such as viewport and interaction, so future migrations should prefer selectors for very narrow read models before expanding the seam again.
+- Any missed consumer of removed implicit APIs will fail at compile time; this is intentional and should be resolved in-code rather than by reintroducing convenience methods.
+
+## Validation
+
+- Passed: `pnpm exec tsc -p tsconfig.app.json --noEmit`
+- Passed: focused post-review regression
+  - `pnpm exec vitest --run src/stores/canvasStore.test.ts src/features/canvas/store/canvasStoreSelectors.test.ts src/features/canvas/hooks/useCanvasExport.test.ts src/features/canvas/canvasPageState.test.ts`
+- Passed: focused canvas regression
+  - `pnpm exec vitest --run src/features/canvas/store/canvasStoreSelectors.test.ts src/features/canvas/canvasPageState.test.ts src/features/canvas/hooks/useCanvasImagePropertyActions.test.ts src/features/canvas/document/commands.test.ts src/features/canvas/document/patches.test.ts src/features/canvas/textSession.test.ts src/features/canvas/renderCanvasDocument.test.ts src/features/canvas/hooks/useCanvasExport.test.ts src/stores/canvasStore.test.ts`
+- Passed: `pnpm lint` with 4 pre-existing `react-refresh/only-export-components` warnings outside this slice
+- Passed: `pnpm test`
+- Passed: `pnpm build:client`
+- Passed: browser smoke on local preview for
+  - create active workbench from header action
+  - switch workbench via workbench list
+  - delete current workbench and recover to the remaining route-bound workbench
+  - open/export dialog and close it again
+- Not automated: asset insertion and canvas-stage undo/redo
+  - current `agent-browser` session can reach shell/panel controls, but the stage interaction path is not stably exposed as an interactive a11y target; regression confidence for those paths comes from the existing automated test suite
+
+## Files
+
+- `src/stores/canvasStore.ts`
+- `src/features/canvas/store/canvasStoreSelectors.ts`
+- `src/features/canvas/hooks/useActiveCanvasWorkbench.ts`
+- `src/features/canvas/store/canvasStoreSelectors.test.ts`
+- first-wave migrated consumers under `src/features/canvas/hooks/*` and `src/features/canvas/CanvasViewport.tsx`
+- `docs/tasks/canvas-active-workbench-boundary.json`

docs/tasks/canvas-active-workbench-usecase-seams.json

@@ -0,0 +1,30 @@
+{
+  "baseline": "a09ba82",
+  "currentTaskId": "4",
+  "tasks": [
+    {
+      "id": "1",
+      "title": "record task artifacts and add active-workbench read/port helpers",
+      "status": "done",
+      "passes": true
+    },
+    {
+      "id": "2",
+      "title": "migrate canvas consumers to state, command, structure, and history seams",
+      "status": "done",
+      "passes": true
+    },
+    {
+      "id": "3",
+      "title": "remove the legacy broad active-workbench facade",
+      "status": "done",
+      "passes": true
+    },
+    {
+      "id": "4",
+      "title": "run focused and full validation and record residual risks",
+      "status": "blocked",
+      "passes": false
+    }
+  ]
+}

docs/tasks/canvas-active-workbench-usecase-seams.md

@@ -0,0 +1,64 @@
+# Canvas Active Workbench Usecase Seams
+
+- Baseline commit: `a09ba82`
+- Branch: `feat/canvas-optimize`
+- Scope: replace the broad active-workbench facade with narrower read-state, command, structure, and history seams without changing store/service persistence semantics or export behavior
+
+## Decisions
+
+- Do not touch export semantics or stage snapshot fallback in this slice.
+- Do not split UI state (`tool`, `zoom`, `viewport`, `activePanel`, `selectedElementIds`) out of `canvasStore` in this slice.
+- Keep `canvasStore` and `canvasWorkbenchService` as the persistence and lifecycle boundary.
+- Remove `useActiveCanvasWorkbench`; replace it with:
+  - `useCanvasActiveWorkbenchState`
+  - `useCanvasActiveWorkbenchCommands`
+  - `useCanvasActiveWorkbenchStructure`
+  - existing `useCanvasHistory`
+- Put the null-safe no-op binding contract in pure helpers under `src/features/canvas/store`, not inside hooks.
+- Keep `useCanvasTextSession` on explicit store APIs because it needs cross-workbench persistence semantics.
+- Migrate only existing active-workbench consumers in this slice; do not widen the new seams for convenience.
+
+## Outcome
+
+- This slice is complete at the canvas-module level: the default active-workbench integration path is now split into read state, command ports, structure ports, and history seams.
+- The old broad `useActiveCanvasWorkbench` facade is removed, and new consumers are expected to bind only the seam they need.
+- Remaining global `tsc` / `test` / `build:client` failures are outside this slice in `image-lab` and `server` conversation routes.
+- After this slice, the only clearly sub-`8.5` canvas architecture hotspot left in the audit is export/render boundary unification.
+
+## Risks
+
+- Consumers that were previously getting mixed read/write capabilities from one facade may end up reassembling those responsibilities if the new seams are not kept narrow.
+- `CanvasViewport` still legitimately mixes read state with explicit text-session store ports; this slice narrows the default path, not every cross-workbench action.
+- `useCanvasHistory` remains a separate seam by design; undo/redo must not drift back into the new command/structure hooks.
+
+## Validation
+
+- Passed focused regression:
+  - `pnpm exec vitest --run src/features/canvas/store/canvasStoreSelectors.test.ts src/features/canvas/store/canvasActiveWorkbenchPorts.test.ts src/stores/canvasStore.test.ts src/features/canvas/hooks/useCanvasImagePropertyActions.test.ts src/features/canvas/textSession.test.ts src/features/canvas/canvasPageState.test.ts src/features/canvas/tools/toolControllers.test.ts`
+  - latest run: `7` files, `61` tests passed
+- Passed broader canvas regression:
+  - `pnpm exec vitest --run src/features/canvas src/stores/canvasStore.test.ts`
+  - latest run: `33` files, `174` tests passed
+- Passed:
+  - `pnpm lint` with 5 existing warnings outside this slice
+- Passed review:
+  - architecture subagent: `no issues found`
+  - bug/regression subagent: `no issues found`
+  - performance subagent: `no issues found`
+- Failed outside this slice:
+  - `pnpm exec tsc -p tsconfig.app.json --noEmit`
+    - first actionable failure: `src/features/image-lab/hooks/useImageGeneration.ts` expects `ImageGenerationResponse.assets` / `runs`
+  - `pnpm test`
+    - first actionable failure: `server/src/routes/image-conversation.test.ts` returns `500` instead of `200` in three conversation route cases
+  - `pnpm build:client`
+    - blocked by the same `image-lab` and `imageConversation` type errors outside the canvas seam
+
+## Files
+
+- `src/features/canvas/store/canvasStoreSelectors.ts`
+- `src/features/canvas/store/canvasActiveWorkbenchPorts.ts`
+- `src/features/canvas/hooks/useCanvasActiveWorkbenchState.ts`
+- `src/features/canvas/hooks/useCanvasActiveWorkbenchCommands.ts`
+- `src/features/canvas/hooks/useCanvasActiveWorkbenchStructure.ts`
+- migrated consumers under `src/features/canvas/hooks/*`, `src/features/canvas/CanvasViewport.tsx`, and `src/features/canvas/hooks/useCanvasExport.ts`
+- `docs/tasks/canvas-active-workbench-usecase-seams.json`