[Spec 945] Foundational reusable package @cluesmith/codev-artifact-canvas

.github/workflows/test.yml

@@ -33,10 +33,18 @@ jobs:
         working-directory: packages/types
         run: pnpm build
 
+      - name: Build artifact-canvas package (+ build-smoke)
+        working-directory: packages/artifact-canvas
+        run: pnpm build && pnpm build:smoke
+
       - name: Run core unit tests
         working-directory: packages/core
         run: pnpm test
 
+      - name: Run artifact-canvas unit tests
+        working-directory: packages/artifact-canvas
+        run: pnpm test
+
       - name: Copy skeleton for unit tests
         working-directory: packages/codev
         run: pnpm copy-skeleton

.gitignore

@@ -31,6 +31,7 @@ packages/codev/dist/
 packages/codev/skeleton/
 packages/types/dist/
 packages/core/dist/
+packages/artifact-canvas/dist/
 packages/vscode/dist/
 packages/vscode/out/
 *.vsix

codev/projects/945-build-foundational-reusable-pa/945-phase_1-iter1-rebuttals.md

@@ -0,0 +1,35 @@
+# Phase 1 (skeleton) — Rebuttal to implement consult iteration 1
+
+**Verdicts:** Claude APPROVE (HIGH); Codex REQUEST_CHANGES (HIGH); Gemini COMMENT (lane skipped).
+Both Codex items were **valid and accepted** — the *code* was correct, but the *spec prose* had
+not been synced to two Phase-1 deferred-item decisions the plan mandated. Both are now fixed.
+
+## Codex — REQUEST_CHANGES (both addressed)
+
+1. **Spec D2 still claimed an "injectable logger."**
+   - **Accepted.** The code already used `console` + the optional `onError?` prop (no logger
+     prop), per plan deferred-item #1. The spec prose was stale.
+   - **Changed** (commit `9d82eaca`): spec D2 now reads "logged to the `console`" + `onError?`,
+     with the "injectable logger" claim removed. While there, I also removed
+     `ThemeAdapter.resolve`/`.onChange` from D2's guarded-calls list, since the v1 component
+     does not call them (D4 Model A) — eliminating a latent D2-vs-D4 contradiction.
+
+2. **Spec `ThemeAdapter.resolve` interface comment showed the ambiguous `("foreground")` example.**
+   - **Accepted.** Code pins the full custom-property name, per plan deferred-item #2.
+   - **Changed** (commit `9d82eaca`): the spec interface comment now shows
+     `resolve("--codev-canvas-foreground")` (full property name).
+
+## Claude — APPROVE
+No changes requested. Claude verified all six interfaces match the spec contracts exactly, the
+dual-format build + tests + smoke pass, and every repo-wiring deliverable is present. No action.
+
+## Gemini — COMMENT (lane skipped)
+The agy lane was skipped (iter-1: timed out producing the review; agy 1.0.7 runs and explores the
+worktree but does not emit a structured verdict within the 5m hardcoded timeout). Non-blocking;
+the precise reason was reported to the architect, who confirmed the timeout constants are not
+env-overridable and cleared a Codex + Claude 2-way for implement-phase advisory consults.
+
+## Post-rebuttal note
+A follow-up self-consult (Codex + Claude) after these fixes returned **Codex COMMENT** (no longer
+REQUEST_CHANGES) and **Claude APPROVE**; Codex's one remaining COMMENT (a stale RC-bump comment in
+the release protocol) was also fixed (commit `73bbbd44`). Net: zero REQUEST_CHANGES outstanding.

codev/projects/945-build-foundational-reusable-pa/945-phase_3-iter1-rebuttals.md

@@ -0,0 +1,40 @@
+# Phase 3 (overlay) — Rebuttal to implement consult iteration 1
+
+**Verdicts:** Codex REQUEST_CHANGES (HIGH); Claude REQUEST_CHANGES (HIGH); Gemini COMMENT.
+Both reviewers converged on the same items — all **accepted and fixed** (commit `106dce84`).
+
+## Codex + Claude (convergent) — accepted & fixed
+
+1. **BUG: `fileAdapter.watch()` was unguarded by try/catch.** A synchronous throw from a host's
+   `watch` would propagate out of the `useEffect` and crash the component, violating the P3 AC
+   "a rejecting `read`/`watch`/`list` is caught → `onError` fired + logged; the component does not
+   throw." **Fixed:** `watch(...)` is now wrapped in try/catch → `report(err)` + a no-op
+   `Disposable` fallback (so the cleanup function still works). Test added (synchronous watch throw
+   → `onError` fired, component renders, no throw).
+
+2. **Missing error/teardown tests against explicit ACs.** **Fixed** — added:
+   - `FileAdapter.read` rejection → `onError` + no throw.
+   - `FileAdapter.watch` synchronous failure → `onError` + no throw.
+   - `Disposable.dispose` called twice → safe no-op (idempotent contract).
+   - Space-key activation (Enter was already covered).
+   Suite is now 26/26 green.
+
+## Codex #2 — marker "author + text via the overlay" (accepted & fixed)
+
+Phase 1's minimal rendering exposed marker author+text only via a `title` tooltip. The plan's
+deferred-#4 wording is "minimal line-level indicator … author + text **via the overlay**."
+**Fixed:** the overlay now renders the active line's markers as a labeled list
+(`author: text`, `aria-label="Comments on line N"`) alongside the `+` affordance, in addition to
+the `.codev-canvas-has-marker` line indicator + `title`. Still minimal v1 (no #863 inline bubbles
+or `<canvas>` minimap). Test added (hovering a marked line surfaces author + text).
+
+## Claude minor (non-blocking) — addressed where cheap
+- `onMouseLeave` on the canvas container now clears the active affordance.
+- Space-key path now tested (was Enter-only).
+- The `+` overlay's visual *positioning* near the hovered block remains a Phase 4 concern (the
+  smoke host is the visual-verification venue); the functional contract (intent fires with the
+  correct line) is met and tested.
+
+## Gemini — COMMENT (non-blocking); no changes required.
+
+Net: both REQUEST_CHANGES resolved; suite 26/26 green; no scope creep into #863/#860–#862.

codev/projects/945-build-foundational-reusable-pa/945-phase_3-iter2-rebuttals.md

@@ -0,0 +1,25 @@
+# Phase 3 (overlay) — Rebuttal to implement consult iteration 2
+
+**Verdicts:** Codex REQUEST_CHANGES (MEDIUM); Claude APPROVE; Gemini COMMENT. Codex's two items
+were **accepted and fixed** (commit `76dfa9a7`). (Claude approved iter-2 — the iter-1 fixes
+satisfied it.)
+
+## Codex — REQUEST_CHANGES (both addressed)
+
+1. **Async race: `read()`/`list()` applied with no sequencing guard.** A slow initial `read()` or an
+   older `list()` could overwrite newer state after a `watch` update (stale content/markers).
+   **Fixed:** added **request-versioning** — a `seqRef` counter; each load (initial read or a watch
+   change) takes a monotonically increasing seq, and content/markers are applied only while that
+   seq is still the latest (`applyLoad` guards before `setContent`, before `setMarkers`, and in the
+   catch). Regression test added: a slow initial `read()` resolving *after* a newer `watch` update
+   does not overwrite the newer content.
+
+2. **The "failed list keeps prior markers" test never rendered prior markers first.** **Fixed:**
+   added a proper test — render a marker, then a *later* `list()` rejects on a `watch` update, and
+   assert the prior marker UI (`.codev-canvas-has-marker`) remains visible.
+
+Tests now 28/28 green.
+
+## Claude — APPROVE; Gemini — COMMENT. No further changes required.
+
+Net: both Codex iter-2 items resolved in `76dfa9a7`; no scope creep.

codev/projects/945-build-foundational-reusable-pa/945-phase_3-iter3-rebuttals.md

@@ -0,0 +1,19 @@
+# Phase 3 (overlay) — Rebuttal to implement consult iteration 3
+
+**Verdicts:** Codex REQUEST_CHANGES (MEDIUM); Claude APPROVE; Gemini COMMENT. Codex's single item
+was **accepted and fixed** (commit `71a2cf7b`).
+
+## Codex — REQUEST_CHANGES (addressed)
+
+- **Out-of-range markers were warned on every `list()` reload, but deferred-#4's AC says
+  "dropped … and warned once."** A noisy `FileAdapter.watch` would re-warn for the same stale
+  marker each refresh. **Fixed:** added a `warnedRef` Set that dedups the `console.warn` per unique
+  stale marker (`line|author|text`), so each stale marker warns exactly once across reloads.
+  Tightened the test to assert `console.warn` is called exactly once even after a `watch` re-list of
+  the same stale marker. 28/28 green.
+
+## Claude — APPROVE; Gemini — COMMENT. No further changes required.
+
+Net: Codex iter-3 item resolved in `71a2cf7b`; no scope creep. (This is the 3rd Codex iteration on
+Phase 3; each item — watch-guard, async race, warn-once — was a legitimate but progressively
+smaller robustness/AC-compliance refinement, all fixed; Claude has APPROVED since iter-2.)

codev/projects/945-build-foundational-reusable-pa/945-phase_3-iter4-rebuttals.md

@@ -0,0 +1,31 @@
+# Phase 3 (overlay) — Rebuttal to implement consult iteration 4
+
+**Verdicts:** Codex REQUEST_CHANGES (HIGH); Claude APPROVE; Gemini COMMENT. Codex's single item was
+**accepted and fixed** (commit `a1fbf0d4`), with explicit architect authorization for the
+contract/spec change it required.
+
+## Codex — REQUEST_CHANGES (addressed)
+
+- **No-watcher refresh contract unmet.** Spec D6 said a host without a `FileAdapter` watcher could
+  "force the same refresh by re-rendering the component," but the effect's stable deps mean a
+  same-props re-render does not re-fetch (which is correct React). Codex is right that the contract
+  as written wasn't honored.
+  - **This required touching the LOCKED `ArtifactCanvasProps` interface**, so I escalated rather than
+    fix unilaterally. The **architect authorized** a purely additive fix.
+  - **Fixed (`a1fbf0d4`):** added an optional `refreshKey?: number | string` to
+    `ArtifactCanvasProps` (no-op default — hosts with a watcher omit it, behavior unchanged) and
+    included it in the effect deps, so a no-watcher host forces a fresh read + marker-list by
+    bumping it. **Spec D6 wording corrected** to name `refreshKey` as the contract (host bumps it on
+    data change; a plain re-render does not re-fetch). Test added: `refreshKey` 1→2 triggers a
+    second read + new content with no watcher involved. 29/29 green.
+
+## Claude — APPROVE; Gemini — COMMENT. No further changes required.
+
+Net: the no-watcher-refresh gap is closed via the additive `refreshKey` prop + the D6 tightening
+(architect-blessed). This is the only iter-4 item.
+
+---
+**Phase 3 consult arc (for the record):** 4 Codex iterations — watch-guard bug, async race,
+warn-once, no-watcher refresh — each a legitimate, progressively smaller refinement, all fixed;
+Claude APPROVED since iter-2; Gemini COMMENT throughout. The architect confirmed the loop was
+working as designed (each iter shipped better software than iter-1 would have).

codev/projects/945-build-foundational-reusable-pa/945-phase_3-iter5-rebuttals.md

@@ -0,0 +1,60 @@
+# Phase 3 (overlay) — Rebuttal to implement consult iteration 5
+
+**Verdicts:** Codex REQUEST_CHANGES (MEDIUM); Claude APPROVE (4th consecutive); Gemini COMMENT.
+Codex raised **two** items. **Item 1 fixed; item 2 resolved as a plan-wording tightening** — both
+with explicit architect authorization (the architect reviewed each on its merits). Commit
+`7261fe16`.
+
+## Codex item 1 — stale `activeLine` after a content reload (ACCEPTED & FIXED)
+
+> "`ArtifactCanvas.tsx:25,152-179` keeps `activeLine` across content refreshes and never validates
+> it against the new document. After a `watch`/`refreshKey` reload that removes or shortens the
+> previously active block, the overlay can still render `+` for a stale line and emit
+> `onAddComment` for a line that no longer exists."
+
+**Legitimate correctness bug — accepted.** `activeLine` (set on hover/focus) survived a `watch`
+reload or `refreshKey` bump without being reconciled against the new content, so a reload that
+removed/shortened the hovered block left the overlay anchored to a line the fresh document no longer
+contains — it could render `+` there and emit `onAddComment(staleLine)`.
+
+**Fixed at root:** a small effect resets `activeLine` to `null` on **every** content change
+(`useEffect(() => setActiveLine(null), [content])`), which covers both the watch-reload and the
+`refreshKey`-bump paths. The user re-hovers/re-focuses to re-anchor the overlay against the fresh
+content. **Regression test added:** hover the last block, trigger a `watch` reload that removes it,
+then assert the `+` affordance is gone and `onAddComment` was **not** called for the removed line.
+30/30 green.
+
+## Codex item 2 — out-of-range marker should also surface via `onError?` (RESOLVED via plan-wording tightening)
+
+> "`ArtifactCanvas.tsx:56-65` only `console.warn`s for out-of-range markers. The phase 3 plan
+> explicitly resolved this policy as 'dropped and reported once via `onError?`/`console.warn`'; the
+> current implementation never surfaces that condition to `onError`… Add the `onError` path and
+> cover it with a test."
+
+**The implementation is correct; the plan wording was ambiguous and has been tightened to match
+it** (architect-authorized, on the same basis as the iter-4 D6 spec tightening — clarifying
+unsettled wording to the semantically-correct shape, not redirecting the plan).
+
+`onError?` is the **adapter-failure** channel — it fires when `read`/`list`/`watch` throw or return
+a rejected promise (and is reserved for #863's `ThemeAdapter.resolve`/`onChange`). An out-of-range
+marker is **data-hygiene during normal rendering**, not a failure: the render succeeded; a stale
+marker was simply dropped. Routing it through `onError?` would force every host to treat a
+non-failure as a failure and would **dilute the signal for genuine failures**. The plan's literal
+`onError?`/`console.warn` phrasing (question-mark-slash) was genuinely ambiguous; it now reads:
+
+> out-of-range markers are reported via **`console.warn` (once per session per marker)**; `onError?`
+> is **reserved for genuine adapter failures** … An out-of-range marker is data-hygiene during
+> normal rendering, not a failure.
+
+The existing warn-once behavior + test (drops, warns exactly once even across reloads) already
+satisfy the tightened policy; no code change for this item.
+
+## Claude — APPROVE (4th); Gemini — COMMENT. No further changes required.
+
+---
+**Phase 3 consult arc (for the record):** 5 Codex iterations — watch-guard bug, async race,
+warn-once, no-watcher refresh, stale-`activeLine` — each a legitimate, progressively smaller
+refinement, all fixed; plus an iter-5 plan-wording tightening (out-of-range channel). Claude
+APPROVED since iter-2; Gemini COMMENT throughout. Each item escalated to the architect when it
+touched a locked contract or the plan text; both iter-5 items were reviewed and authorized on their
+merits before this rebuttal.

codev/projects/945-build-foundational-reusable-pa/945-phase_4-iter1-rebuttals.md

@@ -0,0 +1,27 @@
+# Phase 4 — Rebuttal to implement consult iteration 1
+
+**Verdicts:** Codex REQUEST_CHANGES (HIGH); Claude APPROVE (HIGH); Gemini COMMENT (lane skipped).
+Codex's single item was **accepted and fixed** (commit pending below).
+
+## Codex — REQUEST_CHANGES (accepted & fixed)
+
+> "`README.md:12-21` tells consumers to `pnpm add @cluesmith/codev-artifact-canvas`, but the repo's
+> release protocol explicitly says it is **not independently npm-published in v1** and is consumed
+> via `workspace:*`/host bundling instead (`codev/protocols/release/protocol.md:56`)."
+
+**Legitimate — accepted.** The README's Install section contradicted the locked v1 distribution
+decision (plan iteration 3/4 + `release/protocol.md`: version-aligned but **not** in the
+`pnpm publish` step; consumed via `workspace:*` and bundled by hosts, mirroring
+`@cluesmith/codev-core`).
+
+**Fixed:** the Install section now states the package is not independently npm-published in v1 and
+shows adding it as a `"@cluesmith/codev-artifact-canvas": "workspace:*"` dependency of a host
+package (peers `react`/`react-dom` supplied by the host). No `pnpm add` from npm. The import +
+`default-theme.css` snippets are unchanged (correct once it's a workspace dep). Verified the README
+no longer contains any `pnpm add`/npm-install guidance. Docs-only change — no code touched; build
+33/33 + check-types remain green.
+
+## Claude — APPROVE; Gemini — COMMENT (skipped). No further changes required.
+
+Net: the only iter-1 item (incorrect install guidance) is corrected to match the locked
+workspace-consumption / no-independent-publish decision.

codev/projects/945-build-foundational-reusable-pa/945-review-iter1-rebuttals.md

@@ -0,0 +1,32 @@
+# Review (PR) — Response to PR consult iteration 1
+
+**Verdicts:** Codex REQUEST_CHANGES (HIGH); Claude APPROVE (HIGH); Gemini COMMENT (lane skipped).
+Both Codex items were **accepted and fixed**.
+
+## Codex item 1 — version lockstep (ACCEPTED & FIXED)
+
+> "`packages/artifact-canvas/package.json` is still at `3.1.7` while root, `@cluesmith/codev`,
+> `-core`, `-types`, and `packages/vscode` are all `3.1.9`."
+
+Correct, and a direct consequence of rebasing this branch onto current `main` (which had shipped
+the 3.1.8/3.1.9 releases after this package was created at 3.1.7). The repo's lockstep-version
+invariant — and the release/bump wiring this PR itself adds — require alignment. **Fixed:** bumped
+`@cluesmith/codev-artifact-canvas` to **3.1.9** to match the rest of the workspace. `pnpm install`
+left the lockfile unchanged (workspace package, linked via `workspace:*`); build + 34 tests remain
+green.
+
+## Codex item 2 — approved plan still marked draft (ACCEPTED & FIXED)
+
+> "`codev/plans/945-...md` still says `Status: draft` and has no approval frontmatter, even though
+> `status.yaml` shows `plan-approval.status: approved`."
+
+Correct — the artifact didn't reflect the gate. **Fixed:** added YAML approval frontmatter
+(`approved: 2026-06-10`, `validated: [claude]`, with an approval note mirroring the spec's style and
+referencing the plan-approval gate + the 5-iteration consult history) and flipped the Metadata
+`Status` from `draft` to `approved`.
+
+## Claude — APPROVE (HIGH); Gemini — COMMENT (skipped). No further changes required.
+
+Net: both items are documentation/versioning hygiene with no code-behavior impact; the package
+implementation that Claude approved is unchanged. CI is green (all 6 jobs). Held at the `pr` gate
+for the human.

codev/projects/945-build-foundational-reusable-pa/status.yaml

@@ -0,0 +1,38 @@
+id: '945'
+title: build-foundational-reusable-pa
+protocol: spir
+phase: review
+plan_phases:
+  - id: phase_1
+    title: Package skeleton, dual-format build, locked interfaces, theme tokens
+    status: complete
+  - id: phase_2
+    title: 'Markdown renderer: data-line mapping + DOMPurify sanitization'
+    status: complete
+  - id: phase_3
+    title: Comment overlay (intent-only) + v1 marker rendering + adapter wire-up
+    status: complete
+  - id: phase_4
+    title: Smoke-test host, README, cross-cutting tests
+    status: complete
+current_plan_phase: null
+gates:
+  spec-approval:
+    status: approved
+    requested_at: '2026-05-30T23:25:29.001Z'
+    approved_at: '2026-06-02T06:11:17.264Z'
+  plan-approval:
+    status: approved
+    requested_at: '2026-06-10T00:05:48.982Z'
+    approved_at: '2026-06-10T00:06:53.615Z'
+  pr:
+    status: pending
+    requested_at: '2026-06-10T11:02:06.789Z'
+  verify-approval:
+    status: pending
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-05-30T23:09:36.579Z'
+updated_at: '2026-06-10T11:02:06.790Z'
+pr_ready_for_human: true