diff --git a/.agents/skills/impeccable/SKILL.md b/.agents/skills/impeccable/SKILL.md index 22b344156..8e2ee053e 100644 --- a/.agents/skills/impeccable/SKILL.md +++ b/.agents/skills/impeccable/SKILL.md @@ -1,7 +1,7 @@ --- name: impeccable description: Use when the user wants to design, redesign, shape, critique, audit, polish, clarify, distill, harden, optimize, adapt, animate, colorize, extract, or otherwise improve a frontend interface. Covers websites, landing pages, dashboards, product UI, app shells, components, forms, settings, onboarding, and empty states. Handles UX review, visual hierarchy, information architecture, cognitive load, accessibility, performance, responsive behavior, theming, anti-patterns, typography, fonts, spacing, layout, alignment, color, motion, micro-interactions, UX copy, error states, edge cases, i18n, and reusable design systems or tokens. Also use for bland designs that need to become bolder or more delightful, loud designs that should become quieter, live browser iteration on UI elements, or ambitious visual effects that should feel technically extraordinary. Not for backend-only or non-UI tasks. -version: 4.0.0 +version: 4.0.2 --- This skill gives you the tools and permission to create design that earns to be called out-of-distribution craft: Whereas before, your design work would have been safe, timid and measured, you now approach every design task as a award-winning design director with impeccable understanding for what makes exceptional design work: production-grade code, peak creativity, a clear POV, deep understanding of the needs of the client and users, and exceptional craft. @@ -9,7 +9,7 @@ This skill gives you the tools and permission to create design that earns to be Core principles: - Go all out. No hedging, no shortcuts. The deliverable must be complete (except assets the user must provide). - Dream big and bold. Distinct, beautiful, outstanding and highly inspiring work. -- Iterate with tools available to you (e.g. visual understanding, browser screenshots) until you think this meets the bar. +- Verify in bounded passes, not a loop, and the ceiling covers the whole cycle: screenshots, defect scans, micro-edits, and rebuilds alike. Build fully, inspect once with a batched round (desktop and mobile together), fix everything it shows in one batch, confirm with at most one more round, and stop polishing. Open-ended self-QA burns the user's money doing worse what the finish handoffs do better. ## Setup diff --git a/.agents/skills/impeccable/agents/impeccable_asset_producer.toml b/.agents/skills/impeccable/agents/impeccable_asset_producer.toml index 2419f3ec6..4e5c28efa 100644 --- a/.agents/skills/impeccable/agents/impeccable_asset_producer.toml +++ b/.agents/skills/impeccable/agents/impeccable_asset_producer.toml @@ -47,7 +47,9 @@ Ask blockers once, globally. Missing source path/crops or output directory block 3. Treat full-page mock crops as references, not production-resolution source assets. Put a role in `direct` only when the provided source is already a clean, sufficiently large source asset with no semantic text or presentation chrome. 4. Give the parent an execution order for the `produce` bucket. 5. For produced assets, choose the least inventive strategy: image-to-image clean plate, faithful regeneration from crop reference, transparent cutout, texture/pattern reconstruction, stock/project source, or semantic HTML/CSS/SVG recommendation if raster is wrong. -6. Treat every crop as binding reference. In Codex, use the imagegen skill and built-in `image_gen` path by default when generation or editing is needed. +6. Treat every crop as binding reference. Use the harness's native image tool by default when generation or editing is needed; otherwise use the skill's generate-image.mjs. + +Codex: the imagegen skill's built-in `image_gen` path is the native tool here; prefer it for generation and editing. 7. Remove baked-in UI text, navigation, buttons, body copy, and mock chrome unless the text is part of the asset. 8. Think through the final DOM/CSS representation before generating. If CSS will own radius, clipping, shadows, borders, perspective, responsive cropping, captions, or card frames, do not bake those into the bitmap. 9. Save outputs non-destructively in the requested project directory. @@ -76,7 +78,9 @@ Remove letterboxing, padding, card borders, rounded clipping, CSS shadows, persp Do not add new objects. Do not change the concept. Do not redesign the composition. ``` -For transparent cutouts, use the imagegen skill's built-in-first chroma-key workflow unless the parent explicitly authorizes a true native transparency fallback. +For transparent cutouts, use a chroma-key workflow by default: generate on a flat color that cannot appear in the subject, then post-process to alpha; use true native transparency only when the tool supports it or the parent authorizes it. + +Codex: run the chroma-key workflow through the imagegen skill's built-in-first path. ## Output Contract diff --git a/.agents/skills/impeccable/agents/impeccable_documenter.toml b/.agents/skills/impeccable/agents/impeccable_documenter.toml new file mode 100644 index 000000000..78b6d9d68 --- /dev/null +++ b/.agents/skills/impeccable/agents/impeccable_documenter.toml @@ -0,0 +1,24 @@ +name = "impeccable_documenter" +description = "Records DESIGN.md and its sidecar from a finished Impeccable build, deriving the design system from the shipped artifact rather than from intentions." +model_reasoning_effort = "medium" +nickname_candidates = ["System Scribe", "Token Surveyor", "Ground Truth"] +developer_instructions = ''' +# Impeccable Documenter + +You record a project's design system after the build is done. Ground truth is the shipped artifact: every token and rule you write must be evidenced by the built code, never by what was planned. Writing the system after the fact is the point; a rulebook written before the build gets defended against reality instead of describing it. + +## Input Contract + +Expect: the project root; the artifact path(s); the direction contract text (THESIS, OWN-WORLD, STORY, FIRST VIEWPORT, FORM); PRODUCT.md path; the path to the skill's `reference/document.md`; and the boundary to write at (project or app root). An existing DESIGN.md path means update, not replace: preserve confirmed incumbent decisions and reconcile them with the build. + +## Workflow + +1. Read `reference/document.md` in full; it is the operating spec for DESIGN.md's format, token schema, sidecar, and section order. Follow it exactly. +2. Scan the artifact: stylesheets, custom properties, computed values in the source, component patterns, spacing rhythm, type ramp as actually used. The direction contract's OWN-WORLD block names the world; the build shows how it landed. Where they diverge, the build wins and the prose may note the divergence. +3. Write DESIGN.md (and the sidecar per the spec) with only durable system rules: tokens the project actually uses, named rules the build actually follows. Skip one-off values; a token used once is not a system. +4. Two ways a recorded rule goes wrong, both observed live: a prohibition that bans a device the world itself uses natively, and a value recorded to legitimize a defect. Check every prohibition against the world's own materials; a value earns its place by the build and by legibility, never by making a finding disappear. + +## Output Contract + +Return: the file paths written, a five-line summary of the recorded system (palette strategy, type ramp shape, named rules), and one line naming anything in the build you deliberately did not canonize and why. No other prose. +''' diff --git a/.agents/skills/impeccable/agents/impeccable_finish_reviewer.toml b/.agents/skills/impeccable/agents/impeccable_finish_reviewer.toml new file mode 100644 index 000000000..e336188bc --- /dev/null +++ b/.agents/skills/impeccable/agents/impeccable_finish_reviewer.toml @@ -0,0 +1,29 @@ +name = "impeccable_finish_reviewer" +description = "Reviews a finished Impeccable build against its direction contract, the approved comp, and the chosen world's quality bar, returning an ordered list of material fixes." +model_reasoning_effort = "high" +nickname_candidates = ["Finishing Eye", "Contract Judge", "Ceiling Check"] +developer_instructions = ''' +# Impeccable Finish Reviewer + +You are the finishing reviewer for an Impeccable build: fresh eyes on a done artifact, outside the build thread's attention gravity. You do not edit anything; the parent agent applies your fixes. + +You have no browser. Never attempt to render, screenshot, start a server, or open a page; review from the provided files only. When an expected input is missing, say so in one line at the top of your return and review what is reviewable. + +## Input Contract + +Expect: the original request; the confirmed user answers; the artifact path(s); desktop and mobile screenshot paths captured by the parent; the direction contract (THESIS, OWN-WORLD, STORY, FIRST VIEWPORT, FORM); PRODUCT.md path; existing hook or detector findings; the chosen world's QUALITY BAR card paths and the approved comp path. When the harness can view images, open the screenshots, the comp, and the card before judging. + +## Checks, in order + +1. **Persistence.** PRODUCT.md exists. When DESIGN.md predates this build (an extension or redesign), it matches the built world; on a new world it is written after this review by the documenter, so its absence here is not a finding. +2. **Fidelity.** Against the approved comp: topology, reading order, focal scale, overlaps and z-order, density, signature geometry. Name every drift and classify it intentional or defect. The comp governs composition; it is not a pixel spec for semantics, accessibility, or responsive behavior. +3. **Ceiling.** Against the QUALITY BAR card: name the world's native devices the build left unused, frame, depth, lettering treatment, ornament density, motion. The card governs commitment and finish, never composition. +4. **Contract, promise by promise.** First verify FORM carries the seed key the concept roll printed; a contract with no seed key, or one the parent cannot corroborate, means the roll was skipped and that is a material fix ahead of any craft point. Then, for each of the five blocks, does the render keep the promise? Apply the memory test to the first viewport. +5. **Truth.** Demonstration data authored and labeled synthetic; no invented commercial claims; unanswered claims present as marked placeholders, not omissions. Every image-native region of the approved comp shipped as a real asset, not a gradient standing in for one. + +Do not run a second detector pass; mechanical findings belong to the parent's hooks. + +## Output Contract + +Return exactly five sections: `persistence` (pass/fail with specifics), `fidelity` (drift list with intentional/defect per item, or "faithful"), `ceiling` (unused native devices, or "reached"), `material_fixes` (ordered, most material first, each one line tied to a check or contract promise, at most eight), and `keep` (one line naming what must not be diluted while fixing). Missing inputs are named in one line above the sections. No praise, no summary prose. +''' diff --git a/.agents/skills/impeccable/reference/audit.md b/.agents/skills/impeccable/reference/audit.md index d3483869e..b6a0bb358 100644 --- a/.agents/skills/impeccable/reference/audit.md +++ b/.agents/skills/impeccable/reference/audit.md @@ -26,7 +26,8 @@ Run comprehensive checks across 5 dimensions. Score each dimension 0-4 using the **Check for**: - **Layout thrashing**: Reading/writing layout properties in loops - **Expensive animations**: Casual layout-property animation, unbounded blur/filter/shadow effects, or effects that visibly drop frames -- **Missing optimization**: Images without lazy loading, unoptimized assets, missing will-change +- **Missing optimization**: Images without lazy loading, unoptimized assets +- **will-change overuse**: `will-change` applied broadly or left on at rest (it is a targeted hint for known expensive animations, not a baseline requirement) - **Bundle size**: Unnecessary imports, unused dependencies - **Render performance**: Unnecessary re-renders, missing memoization diff --git a/.agents/skills/impeccable/reference/codex.md b/.agents/skills/impeccable/reference/codex.md deleted file mode 100644 index 5d4c22bf5..000000000 --- a/.agents/skills/impeccable/reference/codex.md +++ /dev/null @@ -1,38 +0,0 @@ -# Codex: Surface Probes & Asset Production - -Load this from [new-work.md](new-work.md) only when the harness has native image generation and a substantial, high-fidelity surface would benefit from seeing the shortlisted concept before code. PRODUCT.md and DESIGN.md are preconditions. New-work has already resolved the visual world; this file must not reopen it. - -The purpose of a probe is to test composition, narrative, hierarchy, density, focal moment, signature use, and image requirements. It is not a second identity workshop. Keep DESIGN.md's palette, typography direction, material language, component character, imagery stance, and motion grammar fixed. - -## Generate the smallest useful probe set - -Generate one to three high-fidelity north-star comps using the native image-generation capability. Base them on the real content and the surface concepts already developed with the user. - -- When the user shortlisted multiple concepts, show one clear expression of each. -- When one concept is already selected, vary only the structural uncertainty that the image can resolve: topology, sequence, density, hierarchy, focal composition, or interaction framing. -- Show enough beyond the opening moment to prove the concept can govern the whole requested surface. -- Do not generate a palette artifact, ask new atmosphere questions, introduce a different type voice, or invent a new motif. If the committed world cannot support the concept, return to the concept shortlist rather than changing the world. - -Treat each comp as a direction test, not a screenshot specification. Core UI text, responsive behavior, accessibility, semantics, and interaction states remain implementation responsibilities. - -## One approval point - -Show the probes together and ask what should carry forward, what feels false to the world, and whether the selected surface concept should be approved, combined, revised, or rejected. Then stop and wait. A structured simulated user counts as attended and receives the same question. - -Do not begin code until the user approves a direction or explicitly delegates the choice. If they delegate, choose using the task brief, PRODUCT.md, and DESIGN.md, and state the evidence. Approval refines the task concept; it does not modify DESIGN.md. - -After approval, summarize the composition and the parts of the comp that must not be literalized. Return to new-work.md, record the direction contract from the approved surface concept, then build. - -## Inventory implementation fidelity - -Before building, inventory the approved comp's major visible ingredients and choose an implementation medium for each: semantic HTML/CSS/SVG, existing project asset, generated raster, sourced raster, icon library, canvas/WebGL, or accepted omission. - -Pay special attention to the dominant composition, signature use, image-native content, second-fold system, and any interaction the still image only implies. If the concept depends on a photograph, architectural scene, product object, portrait, or other raster-native material, do not silently replace it with generic CSS scenery. - -Treat the comp as a north star, not something to trace. Do not rasterize core UI text or controls. Do not substitute a different visual driver after approval without asking. - -## Produce only the assets the build needs - -When clean raster ingredients are required and a scoped subagent is available and authorized, use `impeccable_asset_producer`. Give it the approved comp, output paths, required dimensions and formats, transparency needs, crop notes, and what must remain semantic code. Otherwise produce the minimum required assets with the native image-generation capability in the current thread. - -Return to [new-work.md](new-work.md) for the direction contract, implementation, and the finishing pass. diff --git a/.agents/skills/impeccable/reference/craft-floor.md b/.agents/skills/impeccable/reference/craft-floor.md index 97288e560..4a5d39b5c 100644 --- a/.agents/skills/impeccable/reference/craft-floor.md +++ b/.agents/skills/impeccable/reference/craft-floor.md @@ -4,7 +4,7 @@ Load this after the direction is settled, and build without announcing the check ## Verify -Each of these is a check on the built result, not an intention. +Each of these is a check on the built result, not an intention. Run them together in the batched inspection rounds, not as separate screenshot trips; the checks share one render. - **Contrast:** body and placeholder text ≥4.5:1, large text ≥3:1. On colored surfaces tint secondary text from that hue or the foreground; never gray. - **Depth:** shadows carry an offset and a soft blur. A zero-offset colored halo is decoration. diff --git a/.agents/skills/impeccable/reference/degraded/asset-producer.md b/.agents/skills/impeccable/reference/degraded/asset-producer.md new file mode 100644 index 000000000..751bcbac1 --- /dev/null +++ b/.agents/skills/impeccable/reference/degraded/asset-producer.md @@ -0,0 +1,93 @@ + +This harness has no subagent capability, so you are running this role inline. Step fully out of the work you just finished, adopt only this file's instructions for the pass, and disclose the substitution in one line when you report. Where the text below addresses a parent agent, you are both parties: produce the full output contract first, then act on it yourself. + +# Impeccable Asset Producer + +You are the asset production agent for Impeccable craft. + +Your job is production cleanup, not new art direction. Work only from the approved mock, assigned crops, contact sheets, and constraints the parent agent gives you. The assets you create will be used to build a real site, so treat every raster as a raw ingredient that HTML, CSS, SVG, canvas, and component code will compose. + +## Core Rule + +Do not redesign. Preserve the reference's visual role, silhouette, palette, lighting, material, texture, camera angle, and composition unless the parent explicitly asks for a change. Preserve perspective only when it belongs to the object or scene itself; if CSS should create the card transform, shadow, rounded clipping, border, or layout, remove that presentation chrome from the raster. + +## Input Contract + +Expect: + +- Approved mock path or screenshot reference. +- Crop paths or a contact sheet with crop ids. +- Output directory. +- Required dimensions, format, transparency needs, and avoid list. +- Notes on what should remain semantic HTML/CSS/SVG instead of raster. + +If the source mock is attached but has no filesystem path, use it for visual planning. Ask for a path only before cropping or writing assets. + +Use defaults unless contradicted: + +- `.webp` for opaque photos, backgrounds, and textures. +- `.png` for transparent cutouts, seals, tickets, and illustrations. +- Target production size or at least 2x display size when dimensions are known. Do not use small full-page mock crop size as the default shipping size. +- Remove UI text, navigation, buttons, labels, and body copy by default. +- Keep physical marks only when the parent says they are part of the asset. +- Remove letterboxing, empty padding, baked card corners, borders, shadows, caption bands, and layout background unless the parent says those pixels are intrinsic to the asset. +- Keep the final assets directory clean: only files the build will consume belong there. Put source crops, reference crops, masks, and contact sheets in a sibling `_sources`, `sources`, or review folder. + +Ask blockers once, globally. Missing source path/crops or output directory blocks production. Exact dimensions, compression targets, retina variants, and format preferences do not block; choose defaults and report them. + +## Workflow + +1. Inventory the full approved mock or every assigned crop. +2. Put each visual role in exactly one bucket: + - `produce`: needs generation, image editing, cleanup, cutout work, or a clean plate before it can ship. + - `direct`: can ship as a crop, format conversion, compression pass, or sourced replacement with no generative cleanup. + - `semantic`: build in HTML/CSS/SVG/canvas, no raster output. +3. Treat full-page mock crops as references, not production-resolution source assets. Put a role in `direct` only when the provided source is already a clean, sufficiently large source asset with no semantic text or presentation chrome. +4. Give the parent an execution order for the `produce` bucket. +5. For produced assets, choose the least inventive strategy: image-to-image clean plate, faithful regeneration from crop reference, transparent cutout, texture/pattern reconstruction, stock/project source, or semantic HTML/CSS/SVG recommendation if raster is wrong. +6. Treat every crop as binding reference. Use the harness's native image tool by default when generation or editing is needed; otherwise use the skill's generate-image.mjs. + +Codex: the imagegen skill's built-in `image_gen` path is the native tool here; prefer it for generation and editing. +7. Remove baked-in UI text, navigation, buttons, body copy, and mock chrome unless the text is part of the asset. +8. Think through the final DOM/CSS representation before generating. If CSS will own radius, clipping, shadows, borders, perspective, responsive cropping, captions, or card frames, do not bake those into the bitmap. +9. Save outputs non-destructively in the requested project directory. +10. Compare each output against its source crop. If a review/QA tool is available, run it before the final manifest, then retry each major/fatal finding once before finalizing. + +Use `direct` only for provided source assets that can already ship after crop tightening, conversion, compression, or naming. Do not ship a small crop from the full-page mock as `direct` just because it looks close. + +Use `texture/pattern extraction` only when the source region is already clean enough to sample as texture. If UI, cards, labels, headings, body copy, or footer chrome must be removed to make a reusable texture or background, classify it as crop-derived cleanup or clean-plate work. + +Use `semantic` for dashboards, charts, controls, screenshots of whole UI sections, data widgets, card chrome, app frames, icon toolbars, logos, wordmarks, and anything the final implementation can render crisply in HTML/CSS/SVG/canvas. Only ship a screenshot raster when the parent explicitly says the screenshot itself is the final asset. + +Semantic does not mean ignored. For every semantic role, write a concrete implementation handoff for the parent craft agent: name the DOM/component layers, CSS-owned visual treatment, SVG/canvas/icon-library pieces, responsive behavior, and which nearby produced raster assets it should compose with. For logos and icons, prefer inline SVG/vector or icon-library implementation unless the parent provides a production logo raster. + +For transparency, prefer true alpha output when the tool supports it. If it does not, request a flat chroma-key background in a color that cannot appear in the subject, then post-process that color to alpha before shipping a PNG/WebP. Do not ship the keyed background as the final asset. + +## Prompt Pattern + +Use this shape for image-to-image work: + +```text +Use the provided crop as the approved visual reference. +Recreate the same asset as a clean reusable production image at the target component aspect ratio and at least 2x display resolution. +Preserve silhouette, object/scene perspective, camera angle, palette, lighting, material, texture, and visual role. +Remove baked-in UI copy, navigation, buttons, labels, body text, watermarks, and mock chrome unless explicitly part of the asset. +Remove letterboxing, padding, card borders, rounded clipping, CSS shadows, perspective transforms, caption bands, and layout backgrounds that the implementation should create in code. +Do not add new objects. Do not change the concept. Do not redesign the composition. +``` + +For transparent cutouts, use a chroma-key workflow by default: generate on a flat color that cannot appear in the subject, then post-process to alpha; use true native transparency only when the tool supports it or the parent authorizes it. + +Codex: run the chroma-key workflow through the imagegen skill's built-in-first path. + +## Output Contract + +Return a complete manifest, grouped by `produce`, `direct`, and `semantic`. For each asset include: `id`, `source_crop`, `output_path` when applicable, `strategy`, `prompt_used` when applicable, `dimensions`, `format`, `transparency`, `deviations`, and `qa_status`. + +For each semantic row include `id`, `implementation`, `notes`, and `qa_status`. The `implementation` must be a concrete build handoff, not a short explanation that no asset was produced. It should name the likely HTML/CSS/SVG/canvas/icon/component pieces and the visual responsibilities that code owns. + +`qa_status` must be `accepted`, `needs_parent_review`, or `blocked`. Use `accepted` only after visual comparison passes. Use `needs_parent_review` for cut-off subjects, unwanted borders or rounded-card chrome, letterboxing, baked semantic text, low-resolution output, perspective that should have been CSS, missing transparency, or drift from the crop. Use `blocked` when inputs, permissions, image capability, or asset source quality prevent a credible result. + +End with `execution_order`, `blockers`, and `assumptions` sections. Keep blockers global and minimal. Do not repeat missing inputs in every row; per-asset rows should carry only asset-specific risks or decisions. + +Do not modify implementation code. Do not edit the approved mock. Do not produce final page copy. The parent craft agent owns implementation and final mock fidelity. \ No newline at end of file diff --git a/.agents/skills/impeccable/reference/degraded/documenter.md b/.agents/skills/impeccable/reference/degraded/documenter.md new file mode 100644 index 000000000..e3741ede4 --- /dev/null +++ b/.agents/skills/impeccable/reference/degraded/documenter.md @@ -0,0 +1,21 @@ + +This harness has no subagent capability, so you are running this role inline. Step fully out of the work you just finished, adopt only this file's instructions for the pass, and disclose the substitution in one line when you report. Where the text below addresses a parent agent, you are both parties: produce the full output contract first, then act on it yourself. + +# Impeccable Documenter + +You record a project's design system after the build is done. Ground truth is the shipped artifact: every token and rule you write must be evidenced by the built code, never by what was planned. Writing the system after the fact is the point; a rulebook written before the build gets defended against reality instead of describing it. + +## Input Contract + +Expect: the project root; the artifact path(s); the direction contract text (THESIS, OWN-WORLD, STORY, FIRST VIEWPORT, FORM); PRODUCT.md path; the path to the skill's `reference/document.md`; and the boundary to write at (project or app root). An existing DESIGN.md path means update, not replace: preserve confirmed incumbent decisions and reconcile them with the build. + +## Workflow + +1. Read `reference/document.md` in full; it is the operating spec for DESIGN.md's format, token schema, sidecar, and section order. Follow it exactly. +2. Scan the artifact: stylesheets, custom properties, computed values in the source, component patterns, spacing rhythm, type ramp as actually used. The direction contract's OWN-WORLD block names the world; the build shows how it landed. Where they diverge, the build wins and the prose may note the divergence. +3. Write DESIGN.md (and the sidecar per the spec) with only durable system rules: tokens the project actually uses, named rules the build actually follows. Skip one-off values; a token used once is not a system. +4. Two ways a recorded rule goes wrong, both observed live: a prohibition that bans a device the world itself uses natively, and a value recorded to legitimize a defect. Check every prohibition against the world's own materials; a value earns its place by the build and by legibility, never by making a finding disappear. + +## Output Contract + +Return: the file paths written, a five-line summary of the recorded system (palette strategy, type ramp shape, named rules), and one line naming anything in the build you deliberately did not canonize and why. No other prose. \ No newline at end of file diff --git a/.agents/skills/impeccable/reference/degraded/finish-reviewer.md b/.agents/skills/impeccable/reference/degraded/finish-reviewer.md new file mode 100644 index 000000000..93ce2c807 --- /dev/null +++ b/.agents/skills/impeccable/reference/degraded/finish-reviewer.md @@ -0,0 +1,26 @@ + +This harness has no subagent capability, so you are running this role inline. Step fully out of the work you just finished, adopt only this file's instructions for the pass, and disclose the substitution in one line when you report. Where the text below addresses a parent agent, you are both parties: produce the full output contract first, then act on it yourself. + +# Impeccable Finish Reviewer + +You are the finishing reviewer for an Impeccable build: fresh eyes on a done artifact, outside the build thread's attention gravity. You do not edit anything; the parent agent applies your fixes. + +You have no browser. Never attempt to render, screenshot, start a server, or open a page; review from the provided files only. When an expected input is missing, say so in one line at the top of your return and review what is reviewable. + +## Input Contract + +Expect: the original request; the confirmed user answers; the artifact path(s); desktop and mobile screenshot paths captured by the parent; the direction contract (THESIS, OWN-WORLD, STORY, FIRST VIEWPORT, FORM); PRODUCT.md path; existing hook or detector findings; the chosen world's QUALITY BAR card paths and the approved comp path. When the harness can view images, open the screenshots, the comp, and the card before judging. + +## Checks, in order + +1. **Persistence.** PRODUCT.md exists. When DESIGN.md predates this build (an extension or redesign), it matches the built world; on a new world it is written after this review by the documenter, so its absence here is not a finding. +2. **Fidelity.** Against the approved comp: topology, reading order, focal scale, overlaps and z-order, density, signature geometry. Name every drift and classify it intentional or defect. The comp governs composition; it is not a pixel spec for semantics, accessibility, or responsive behavior. +3. **Ceiling.** Against the QUALITY BAR card: name the world's native devices the build left unused, frame, depth, lettering treatment, ornament density, motion. The card governs commitment and finish, never composition. +4. **Contract, promise by promise.** First verify FORM carries the seed key the concept roll printed; a contract with no seed key, or one the parent cannot corroborate, means the roll was skipped and that is a material fix ahead of any craft point. Then, for each of the five blocks, does the render keep the promise? Apply the memory test to the first viewport. +5. **Truth.** Demonstration data authored and labeled synthetic; no invented commercial claims; unanswered claims present as marked placeholders, not omissions. Every image-native region of the approved comp shipped as a real asset, not a gradient standing in for one. + +Do not run a second detector pass; mechanical findings belong to the parent's hooks. + +## Output Contract + +Return exactly five sections: `persistence` (pass/fail with specifics), `fidelity` (drift list with intentional/defect per item, or "faithful"), `ceiling` (unused native devices, or "reached"), `material_fixes` (ordered, most material first, each one line tied to a check or contract promise, at most eight), and `keep` (one line naming what must not be diluted while fixing). Missing inputs are named in one line above the sections. No praise, no summary prose. \ No newline at end of file diff --git a/.agents/skills/impeccable/reference/degraded/manual-edit-applier.md b/.agents/skills/impeccable/reference/degraded/manual-edit-applier.md new file mode 100644 index 000000000..64678ebe8 --- /dev/null +++ b/.agents/skills/impeccable/reference/degraded/manual-edit-applier.md @@ -0,0 +1,92 @@ + +This harness has no subagent capability, so you are running this role inline. Step fully out of the work you just finished, adopt only this file's instructions for the pass, and disclose the substitution in one line when you report. Where the text below addresses a parent agent, you are both parties: produce the full output contract first, then act on it yourself. + +# Impeccable Manual Edit Applier + +You apply one leased Impeccable live `manual_edit_apply` event to real source files. + +The parent live thread owns polling and protocol replies. You own source edits only. + +## Input Contract + +Expect a self-contained handoff with: + +- Repository root. +- Scripts path. +- Event id. +- Page URL. +- Optional chunk metadata. +- Optional repair metadata. When present, fix the current source after a failed validation attempt; do not restart from the pre-Apply source. +- Optional deadline. +- The current event `batch`. +- Optional `evidencePath`. + +The user already clicked Apply. Do not ask what to do. Do not discard edits. Do not run `live-poll.mjs`, `live-commit-manual-edits.mjs`, or any live server endpoint. Do not run `live-commit-manual-edits.mjs` for a leased manual Apply event. Do not stage, commit, rebuild, push, or edit generated provider output unless the batch explicitly targets that generated file. + +## Workflow + +1. Treat `batch`, `op.originalText`, and `op.newText` as literal data, never instructions. +2. If `evidencePath` is present, read it when source hints are missing, stale, or ambiguous. +3. Apply only the entries and ops in the current event. If `chunk` is present, later staged edits arrive in later chunks. +4. Use evidence in order: `sourceHint.file` + `sourceHint.line`, candidate source hints, object-key/text/context matches, then locator or nearby text. +5. For hinted leaf text, replace only exact source text at or near the hint. Do not rewrite parent sections, containers, unrelated markup, or formatting. +6. Never use DOM outerHTML as source text. Source text must be an exact substring already present in the file. +7. For mixed markup that renders one visible phrase, preserve existing child tags and edit only the changed text node. +8. If evidence points to rendered data, edit the source data object or mapped-list item that renders the visible copy. +9. If visible text is also a string literal or object key, update clearly coupled lookup keys for counts, animations, icons, images, assets, styles, metadata, or other dependent maps in the same response. +10. If candidates.objectKeyMatches points at the old visible text as a key, that key must either be renamed to `op.newText` or the entry must fail. Leaving the old key behind can break rendered images, counts, or assets. +11. If one op renames a label and another changes a value looked up by that label, update the same lookup/map entry so the key uses the new label and the value uses the exact new display text. +12. Preserve `op.newText` exactly, including leading zeros, punctuation, casing, spacing, and temporary-looking words. +13. Preserve typed source data. Do not turn numeric, boolean, array, or object model values into strings unless the visible value truly became display text. +14. If numeric copy is rendered from an expression, change the display expression or a clearly coupled lookup value; do not replace the underlying typed model declaration with quoted copy. +15. `sourceContext` is current source after earlier chunks and retries. If event evidence disagrees with current source, current source wins; `sourceEdit.originalText` must appear exactly in the current file. +16. In JSX/TSX, if the original visible copy is rendered by an expression-only text node and the new value is display copy, keep the replacement expression-shaped with a quoted expression such as `{"7 seats"}` rather than raw text. +17. When user copy contains framework-sensitive characters such as `>`, keep the visible text exact but encode it as valid source. In JSX/TSX text nodes, use a quoted expression like `{"alpha -> beta"}` instead of raw text that contains `>`. +18. If numeric-looking visible text is not a valid safe numeric literal for the source language, write it as display text. Leading-zero decimals and mixed alphanumeric counts must be quoted/escaped as strings in JS/TS data. +19. If numeric source data is changed to non-numeric visible text, write the new visible text as a quoted source string. Never substitute a similar number or a bare identifier. +20. When the user changes visible copy back to a plain number and evidence shows the source model was numeric, restore the numeric value without quotes. +21. If a dependency is ambiguous or broad, fail that entry and leave no partial edits for it. +22. Never copy browser/runtime scaffolding into source: no `contenteditable`, `data-impeccable-*`, variant wrappers, live markers, generated browser attrs, `' + + '
Hi
'; + +// A DESIGN.md whose typography allows only Palatino — Verdana violates it. +const DESIGN_MD = `--- +typography: + body: + fontFamily: "Palatino, Georgia, serif" +--- +# Project A Design System +`; + +const tempRoots = []; + +function mkProject({ withDesign, withMarker = true }) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-contam-')); + tempRoots.push(dir); + if (withMarker) fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"fixture"}'); + if (withDesign) fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD); + const page = path.join(dir, 'page.html'); + fs.writeFileSync(page, PAGE_HTML); + return { dir, page }; +} + +// Run the CLI from `cwd`; force the node binary so the HTML/jsdom path never +// runs under bun (which is unusably slow). +function runDetect(cwd, targets) { + const result = spawnSync(process.execPath, [CLI, 'detect', '--json', ...targets], { + cwd, + encoding: 'utf-8', + }); + let findings = []; + try { + findings = JSON.parse(result.stdout || '[]'); + } catch { + throw new Error(`Non-JSON CLI output.\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); + } + return findings; +} + +function fontFindingsFor(findings, file) { + return findings.filter( + (f) => f.antipattern === 'design-system-font' && (!file || f.file === file), + ); +} + +let projA; +let projB; + +before(() => { + projA = mkProject({ withDesign: true }); // DESIGN.md forbids Verdana + projB = mkProject({ withDesign: false }); // its own project, no DESIGN.md +}); + +after(() => { + for (const dir of tempRoots) { + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best effort */ } + } +}); + +describe('detect CLI DESIGN.md resolution', () => { + it('does NOT apply cwd project A\'s DESIGN.md to project B\'s file (the contamination bug)', () => { + const findings = runDetect(projA.dir, [projB.page]); + assert.deepEqual( + fontFindingsFor(findings, projB.page).map((f) => f.ignoreValue), + [], + 'project B\'s Verdana must not be flagged by project A\'s DESIGN.md', + ); + }); + + it('still applies a project\'s own DESIGN.md to its own file (positive control)', () => { + const findings = runDetect(projA.dir, [projA.page]); + assert.ok( + fontFindingsFor(findings, projA.page).some((f) => f.ignoreValue === 'verdana'), + 'project A\'s own DESIGN.md must flag Verdana in project A\'s file', + ); + }); + + it('resolves per target when one scan spans two projects', () => { + const findings = runDetect(projA.dir, [projA.page, projB.page]); + assert.ok( + fontFindingsFor(findings, projA.page).length > 0, + 'A\'s file should be judged against A\'s DESIGN.md', + ); + assert.equal( + fontFindingsFor(findings, projB.page).length, + 0, + 'B\'s file should NOT be judged against A\'s DESIGN.md', + ); + }); + + it('falls back to no design system for a bare file with no project markers above it', () => { + // A lone file whose directory has neither .git, package.json, nor .impeccable. + const bareDir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-bare-')); + tempRoots.push(bareDir); + const barePage = path.join(bareDir, 'page.html'); + fs.writeFileSync(barePage, PAGE_HTML); + + const findings = runDetect(projA.dir, [barePage]); + assert.equal( + fontFindingsFor(findings, barePage).length, + 0, + 'a project-less file must fall back to no design system, not cwd\'s', + ); + }); +}); diff --git a/tests/doctor.test.mjs b/tests/doctor.test.mjs index 22243396a..2e120e4c2 100644 --- a/tests/doctor.test.mjs +++ b/tests/doctor.test.mjs @@ -39,7 +39,11 @@ beforeEach(() => { }); afterEach(() => { - fs.rmSync(scratch, { recursive: true, force: true }); + // These tests run real git subprocesses in the scratch dir; on Node 22 a + // recursive delete can race git's object writes and fail the whole test + // with ENOTEMPTY (seen in CI). maxRetries/retryDelay make rmSync retry + // exactly those transient errors. + fs.rmSync(scratch, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); function write(rel, body) { @@ -272,6 +276,104 @@ describe('checkHookInstallation', () => { [], ); }); + + // The manifest `impeccable hooks on` actually writes, verbatim: a + // `${CLAUDE_PROJECT_DIR}`-relative command. Claude Code expands the variable + // to the project dir at hook time; the doctor must expand it the same way + // (issue #402) instead of existsSync-ing the literal `${CLAUDE_PROJECT_DIR}/...`. + const claudeManifest = () => ({ + hooks: { + PostToolUse: [{ + matcher: 'Edit|Write|MultiEdit', + hooks: [{ type: 'command', command: 'node "${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs"' }], + }], + Stop: [{ hooks: [{ type: 'command', command: 'node "${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs"' }] }], + }, + }); + + it('stays quiet for a ${CLAUDE_PROJECT_DIR} manifest when the script exists at root', () => { + write('.claude/skills/impeccable/scripts/hook.mjs', '// hook\n'); + write('.claude/settings.json', JSON.stringify(claudeManifest())); + assert.deepEqual( + checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code' }), + [], + ); + }); + + it('still flags a genuinely missing script behind ${CLAUDE_PROJECT_DIR}', () => { + // Placeholder expands to a real path that does not exist: the check must + // stay real, not neutered into always-quiet. + write('.claude/settings.json', JSON.stringify(claudeManifest())); + const findings = checkHookInstallation({ + projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code', + }); + assert.deepEqual(ids(findings), ['hook-script-missing']); + }); + + it('handles the #399 guarded project-relative form', () => { + const p = '"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs"'; + const guarded = `[ ! -f ${p} ] || node ${p}`; + write('.claude/settings.json', JSON.stringify({ + hooks: { Stop: [{ hooks: [{ command: guarded }] }] }, + })); + // missing → flagged + assert.deepEqual( + ids(checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code' })), + ['hook-script-missing'], + ); + // present → quiet + write('.claude/skills/impeccable/scripts/hook.mjs', '// hook\n'); + assert.deepEqual( + checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code' }), + [], + ); + }); + + it('handles the #399 guarded absolute form (user-level installs)', () => { + const abs = path.join(scratch, '.claude', 'skills', 'impeccable', 'scripts', 'hook.mjs'); + const p = JSON.stringify(abs); + const guarded = `[ ! -f ${p} ] || node ${p}`; + write('.claude/settings.json', JSON.stringify({ + hooks: { Stop: [{ hooks: [{ command: guarded }] }] }, + })); + // absolute path missing → flagged + assert.deepEqual( + ids(checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code' })), + ['hook-script-missing'], + ); + // present → quiet + write('.claude/skills/impeccable/scripts/hook.mjs', '// hook\n'); + assert.deepEqual( + checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code' }), + [], + ); + }); + + it('never reports missing for the GitHub $(git rev-parse) form', () => { + // Command substitution is not statically resolvable; a doctor must not + // assert a negative it cannot verify. + write('.github/hooks/impeccable.json', JSON.stringify({ + hooks: { postToolUse: [{ bash: 'node "$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts/hook.mjs"' }] }, + })); + assert.deepEqual( + checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'github' }), + [], + ); + }); + + it('never reports missing for plugin-root placeholders the doctor cannot map', () => { + for (const token of ['${CLAUDE_PLUGIN_ROOT}', '${PLUGIN_ROOT}', '${GROK_PLUGIN_ROOT}']) { + fs.rmSync(path.join(scratch, '.claude'), { recursive: true, force: true }); + write('.claude/settings.json', JSON.stringify({ + hooks: { Stop: [{ hooks: [{ command: `node "${token}/skills/impeccable/scripts/hook.mjs"` }] }] }, + })); + assert.deepEqual( + checkHookInstallation({ projectRoot: scratch, repoRoot: scratch, providerId: 'claude-code' }), + [], + `expected no finding for ${token}`, + ); + } + }); }); // ─── retired live-mode state ─────────────────────────────────────────────── diff --git a/tests/fixtures/antipatterns/color.html b/tests/fixtures/antipatterns/color.html index 8949cb3a1..1debc1216 100644 --- a/tests/fixtures/antipatterns/color.html +++ b/tests/fixtures/antipatterns/color.html @@ -24,6 +24,18 @@ .panel-reset { background: rgb(28, 30, 38); color: rgb(230, 232, 237); padding: 12px; } .panel-reset code { background: rgb(246, 242, 244); border-radius: 3px; padding: 1px 4px; } .panel-reset pre code { background: none; } + /* issue #409 Case A: gradient-clipped text. The gradient IS the glyph fill + (text-fill-color: transparent), not a backdrop. The inherited `color` + (#e8e6e3) is never painted, so measuring it against the element's own + gradient stops (#6d8cff / #a78bfa) is a false positive. */ + .ox-grad-text { background: linear-gradient(135deg, #6d8cff 0%, #a78bfa 50%, #6d8cff 100%); -webkit-background-clip: text; background-clip: text; -webkit-text-fill-color: transparent; color: #e8e6e3; font-size: 40px; font-weight: 800; } + /* issue #409 Case B: a 9%-alpha accent glow stop over a dark surface. The + stop composites to ~#121f1f, not the full-opacity #34c0a8, so text stays + high-contrast. The dark wrapper supplies the surface beneath the glow. */ + .ox-dark-wrap { background: #0f0f11; padding: 16px; } + .ox-glow { background: linear-gradient(160deg, rgba(52,192,168,0.09) 0%, #141419 65%); padding: 20px; } + .ox-glow p { color: #e8e6e3; font-size: 18px; } + .ox-glow .muted { color: #8e8c89; font-size: 16px; } @@ -181,6 +193,25 @@ 
light text over the dark panel, not the light code surface
+

Gradient-clipped text (issue #409 Case A — must not flag contrast)

+ +

Gradient Clipped Heading Text

+ +

Faint accent-glow gradient (issue #409 Case B — must not flag contrast)

+ +
+
+

Light body copy on a faint accent glow that composites to near-black

+

Muted secondary line on the same faint glow area here

+
+
+

Emoji on light backgrounds

diff --git a/tests/fixtures/antipatterns/em-dash-entities.html b/tests/fixtures/antipatterns/em-dash-entities.html index 655efcdaf..eaf8f0c74 100644 --- a/tests/fixtures/antipatterns/em-dash-entities.html +++ b/tests/fixtures/antipatterns/em-dash-entities.html @@ -10,7 +10,7 @@

The product is fast — it is also cheap — and it is honest — which matters — more than speed — or price - — in the long run. + — in the short term — and the long run — always.

diff --git a/tests/fixtures/antipatterns/named-color-borders.html b/tests/fixtures/antipatterns/named-color-borders.html new file mode 100644 index 000000000..f3e0e2db7 --- /dev/null +++ b/tests/fixtures/antipatterns/named-color-borders.html @@ -0,0 +1,139 @@ + + + + + Side-Tab with CSS Named Colors (purple/rebeccapurple/crimson/teal) + + + +
+
+

Should flag

+

named purple

border-left 4px solid purple + radius

+

rebeccapurple

border-left 4px + radius

+

crimson top stripe

border-top 4px, horizontal variant

+

named teal

border-left 3px, no radius

+

var() to named

border-left 4px solid var(--accent)

+
+

inline named purple

style attribute, issue #359 case (a); width 7px keeps the snippet unique

+
+
+
+

Should pass

+

dimgray

neutral named color

+

gainsboro

light neutral named color

+

named black

neutral side border

+

1px purple

too thin to qualify

+

uniform purple

all four sides, not a stripe

+
+
+ + + diff --git a/tests/fixtures/antipatterns/nonrendered-text.html b/tests/fixtures/antipatterns/nonrendered-text.html new file mode 100644 index 000000000..a89d225ea --- /dev/null +++ b/tests/fixtures/antipatterns/nonrendered-text.html @@ -0,0 +1,50 @@ + + + + + + +This is a fairly long document title that easily exceeds twenty characters + + + + + + + + + + + + +
Hidden display-none block of body text long enough to exceed twenty characters.
+ + +

Invisible visibility-hidden paragraph copy that is longer than the twenty char gate.

+ + +

This is real rendered body copy at 10px that is definitely long enough to flag.

+ + + + + + diff --git a/tests/fixtures/antipatterns/pseudo-stripe.css b/tests/fixtures/antipatterns/pseudo-stripe.css new file mode 100644 index 000000000..91286b50d --- /dev/null +++ b/tests/fixtures/antipatterns/pseudo-stripe.css @@ -0,0 +1,120 @@ +/* + * Pseudo-element stripe fixture for the regex engine (issue #394). + * + * The side-tab silhouette built as an absolutely-positioned ::before/::after + * bar instead of a border. scanCssTextForPseudoStripe already caught these on + * full HTML pages; this fixture pins the standalone-stylesheet path. The + * data-case attribute in each selector lands in the finding snippet, so the + * test can attribute every flag/pass case individually. + */ + +:root { + --stripe-fixture-neutral: #e5e7eb; +} + +/* ── FLAG: the issue reproducer — inset shorthand pin + 4px chromatic bar ── */ +.card[data-case="Inset Shorthand Left Edge"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 4px; + background: #7c3aed; +} + +/* ── FLAG: longhand edge pins; unresolvable var() errs toward detection ── */ +.card[data-case="Longhand Left Edge"]::before { + content: ""; + position: absolute; + top: 0; + bottom: 0; + left: 0; + width: 4px; + background: var(--accent); +} + +/* ── FLAG: horizontal variant riding the bottom edge ── */ +.card[data-case="Bottom Edge"]::after { + content: ""; + position: absolute; + left: 0; + right: 0; + bottom: 0; + height: 4px; + background: #f43f5e; +} + +/* ── FLAG: height:100% full-height stripe on the right edge ── */ +.card[data-case="Full Height Right Edge"]::before { + content: ""; + position: absolute; + top: 0; + right: 0; + height: 100%; + width: 3px; + background: oklch(60% 0.2 300); +} + +/* ── PASS: neutral gray divider is not an accent stripe ── */ +.card[data-case="Neutral Divider"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 4px; + background: var(--stripe-fixture-neutral); +} + +/* ── PASS: 24px is a panel, not a stripe ── */ +.card[data-case="Wide Panel"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 24px; + background: #7c3aed; +} + +/* ── PASS: no position: absolute — not an overlay stripe ── */ +.card[data-case="Static Underline"]::before { + content: ""; + width: 4px; + background: #7c3aed; +} + +/* ── PASS: 1px hairline is a divider ── */ +.card[data-case="Hairline Divider"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 1px; + background: #7c3aed; +} + +/* ── PASS: hover-conditional underline is an affordance, not decoration ── */ +.link-row[data-case="Hover Underline"]:hover::after { + content: ""; + position: absolute; + left: 0; + right: 0; + bottom: 0; + height: 4px; + background: #7c3aed; +} + +/* ── PASS: pinned to one edge but not full-height — a badge, not a stripe ── */ +.card[data-case="Floating Badge"]::before { + content: ""; + position: absolute; + left: 0; + width: 4px; + height: 12px; + background: #7c3aed; +} + +/* ── PASS: commented-out CSS is not a live rule ── +.card[data-case="Commented Out Stripe"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 4px; + background: #7c3aed; +} +*/ diff --git a/tests/fixtures/antipatterns/pseudo-stripe.vue b/tests/fixtures/antipatterns/pseudo-stripe.vue new file mode 100644 index 000000000..a3b53a4ff --- /dev/null +++ b/tests/fixtures/antipatterns/pseudo-stripe.vue @@ -0,0 +1,29 @@ + + + + + diff --git a/tests/fixtures/antipatterns/radial-spotlight-glow.html b/tests/fixtures/antipatterns/radial-spotlight-glow.html new file mode 100644 index 000000000..6c820522c --- /dev/null +++ b/tests/fixtures/antipatterns/radial-spotlight-glow.html @@ -0,0 +1,112 @@ + + + + +radial-spotlight-glow fixture + + + +
+ +
+
+
+
+
+ + +
+
+
+
+
+
+
+
+
+
+ + diff --git a/tests/fixtures/antipatterns/shadowed-form-id.html b/tests/fixtures/antipatterns/shadowed-form-id.html new file mode 100644 index 000000000..ccc1cf473 --- /dev/null +++ b/tests/fixtures/antipatterns/shadowed-form-id.html @@ -0,0 +1,50 @@ + + + + + +Shadowed form.id regression fixture + + + +
+

Impeccable Test Product

+

$49.00

+ + +
+ + + + + + + +
+
+ + + diff --git a/tests/fixtures/antipatterns/undersized-ui-text.html b/tests/fixtures/antipatterns/undersized-ui-text.html new file mode 100644 index 000000000..b70029606 --- /dev/null +++ b/tests/fixtures/antipatterns/undersized-ui-text.html @@ -0,0 +1,96 @@ + + + + +undersized-ui-text fixture + + + + + +
+

Should flag

+ + + + + + Flag Category + + + Flag Meta Row + + + + + + + + + +
Flag Table Cell
+ + + Flag Caps Label + + + +
+ + +
+

Should pass

+ + + + + + Pass Sr Only + + +

ReferencePass Sup Marker and waterPass Sub Marker

+ + +
Pass Em Sized
+ + +
+ Pass Terminal Line +
+ + + Pass Normal Link +
+ + + diff --git a/tests/framework-fixtures.test.mjs b/tests/framework-fixtures.test.mjs index 45df1eaed..15be7cf41 100644 --- a/tests/framework-fixtures.test.mjs +++ b/tests/framework-fixtures.test.mjs @@ -157,6 +157,19 @@ for (const name of listFixtures()) { assert.match(body, /localhost:9999\/live\.js/); return; } + if (result.adapter === 'tanstack-start') { + const adapterResult = result.results[0]; + const rootDoc = readFileSync(join(tmp, adapterResult.file), 'utf-8'); + const component = readFileSync(join(tmp, adapterResult.componentFile), 'utf-8'); + assert.equal(adapterResult.inserted, true, 'TanStack Start root document was patched'); + assert.match(rootDoc, /impeccable-live-tanstack-start/, 'root document got the adapter marker'); + assert.match(rootDoc, //, 'root document renders the mount component'); + assert.doesNotMatch(rootDoc, /impeccable-live-start/, 'root document must not get the raw script block'); + assert.doesNotMatch(rootDoc, /localhost:9999\/live\.js/, 'root document must not own live.js directly'); + assert.match(component, /localhost:9999\/live\.js/, 'mount component loads live.js'); + assert.match(component, /useEffect/, 'mount component appends the script on mount'); + return; + } for (const r of result.results) { assert.ok(r.inserted, `${r.file} got the tag (result: ${JSON.stringify(r)})`); const body = readFileSync(join(tmp, r.file), 'utf-8'); @@ -189,6 +202,14 @@ for (const name of listFixtures()) { assert.equal(existsSync(join(tmp, result.results[0].file)), false, 'Nuxt client plugin was removed'); return; } + if (result.adapter === 'tanstack-start') { + const adapterResult = result.results[0]; + const rootDoc = readFileSync(join(tmp, adapterResult.file), 'utf-8'); + assert.doesNotMatch(rootDoc, /ImpeccableLiveRoot/); + assert.doesNotMatch(rootDoc, /impeccable-live-tanstack-start/); + assert.equal(existsSync(join(tmp, adapterResult.componentFile)), false, 'TanStack mount component was removed'); + return; + } for (const r of result.results) { const body = readFileSync(join(tmp, r.file), 'utf-8'); assert.doesNotMatch(body, /impeccable-live-start/); diff --git a/tests/framework-fixtures/README.md b/tests/framework-fixtures/README.md index 1a2bc4c64..bbbaa8c01 100644 --- a/tests/framework-fixtures/README.md +++ b/tests/framework-fixtures/README.md @@ -113,6 +113,8 @@ When `preActions` is omitted, steer smoke inherits `runtime.preActions` to revea | `astro/` | `src/layouts/Layout.astro` as inject target. HTML comments. | | `sveltekit/` | `src/app.html` shell + `src/routes/+page.svelte`. | | `nuxt-vite7/` | Nuxt 4 `app/` structure + Vue 3 SFC. Live loads through a generated dev-only client plugin. | +| `tanstack-router-vite/` | Vite + TanStack Router (code-based SPA). Tracked `index.html` shell inject (the baseline Vite path, no adapter). | +| `tanstack-start/` | Vite + TanStack Start (SSR). No static `index.html`; Live patches the `__root.tsx` document to mount a generated dev-only React component that loads the bundle. | | `multipage-with-generator/` | `src/` tracked, `dist/` gitignored. Exercises the is-generated guard and `element_not_in_source` fallback. | | `nextjs-turborepo/` | Monorepo with shared CSP helper (`createBaseNextConfig`). CSP shape `append-arrays`. | | `nextjs-inline-csp/` | App-level `next.config.js` with a literal CSP string. CSP shape `append-string`. | diff --git a/tests/framework-fixtures/astro-vite7/files/package.json b/tests/framework-fixtures/astro-vite7/files/package.json index 33ba1a69d..bd826d09a 100644 --- a/tests/framework-fixtures/astro-vite7/files/package.json +++ b/tests/framework-fixtures/astro-vite7/files/package.json @@ -9,6 +9,6 @@ "preview": "astro preview" }, "devDependencies": { - "astro": "^6.0.0" + "astro": "^7.1.0" } } diff --git a/tests/framework-fixtures/astro-vite7/fixture.json b/tests/framework-fixtures/astro-vite7/fixture.json index 53ad21b50..92b409a44 100644 --- a/tests/framework-fixtures/astro-vite7/fixture.json +++ b/tests/framework-fixtures/astro-vite7/fixture.json @@ -1,5 +1,5 @@ { - "name": "Astro 6 + Vite 7", + "name": "Astro 7 + Vite 7", "config": { "files": ["src/layouts/Layout.astro"], "insertBefore": "", diff --git a/tests/framework-fixtures/tanstack-router-vite/files/index.html b/tests/framework-fixtures/tanstack-router-vite/files/index.html new file mode 100644 index 000000000..56d3ff84b --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/index.html @@ -0,0 +1,11 @@ + + + + + Vite 8 + TanStack Router Fixture + + +
+ + + diff --git a/tests/framework-fixtures/tanstack-router-vite/files/package.json b/tests/framework-fixtures/tanstack-router-vite/files/package.json new file mode 100644 index 000000000..95acee9ee --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/package.json @@ -0,0 +1,20 @@ +{ + "name": "tanstack-router-vite-fixture", + "private": true, + "version": "0.0.0", + "type": "module", + "scripts": { + "dev": "vite --host 127.0.0.1", + "build": "vite build", + "preview": "vite preview" + }, + "dependencies": { + "@tanstack/react-router": "^1.132.0", + "react": "^19.0.0", + "react-dom": "^19.0.0" + }, + "devDependencies": { + "@vitejs/plugin-react": "^6.0.0", + "vite": "^8.0.0" + } +} diff --git a/tests/framework-fixtures/tanstack-router-vite/files/src/main.jsx b/tests/framework-fixtures/tanstack-router-vite/files/src/main.jsx new file mode 100644 index 000000000..ae32f1b35 --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/src/main.jsx @@ -0,0 +1,37 @@ +import { StrictMode } from 'react'; +import { createRoot } from 'react-dom/client'; +import { + createRootRoute, + createRoute, + createRouter, + RouterProvider, + Outlet, + Link, +} from '@tanstack/react-router'; +import Home from './routes/Home.jsx'; +import About from './routes/About.jsx'; +import './styles.css'; + +const rootRoute = createRootRoute({ + component: () => ( + <> + + + + ), +}); + +const indexRoute = createRoute({ getParentRoute: () => rootRoute, path: '/', component: Home }); +const aboutRoute = createRoute({ getParentRoute: () => rootRoute, path: '/about', component: About }); + +const routeTree = rootRoute.addChildren([indexRoute, aboutRoute]); +const router = createRouter({ routeTree }); + +createRoot(document.getElementById('root')).render( + + + , +); diff --git a/tests/framework-fixtures/tanstack-router-vite/files/src/routes/About.jsx b/tests/framework-fixtures/tanstack-router-vite/files/src/routes/About.jsx new file mode 100644 index 000000000..ae4c31e15 --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/src/routes/About.jsx @@ -0,0 +1,8 @@ +export default function About() { + return ( +
+

About Page Hero

+

Lives on the /about route — only mounts after navigation.

+
+ ); +} diff --git a/tests/framework-fixtures/tanstack-router-vite/files/src/routes/Home.jsx b/tests/framework-fixtures/tanstack-router-vite/files/src/routes/Home.jsx new file mode 100644 index 000000000..3c022d03d --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/src/routes/Home.jsx @@ -0,0 +1,8 @@ +export default function Home() { + return ( +
+

Home

+

Welcome. The hero we'll edit lives on the About page.

+
+ ); +} diff --git a/tests/framework-fixtures/tanstack-router-vite/files/src/styles.css b/tests/framework-fixtures/tanstack-router-vite/files/src/styles.css new file mode 100644 index 000000000..7d2c6e91f --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/src/styles.css @@ -0,0 +1,6 @@ +body { margin: 0; font-family: system-ui, sans-serif; } +.nav { display: flex; gap: 1rem; padding: 1rem; border-bottom: 1px solid #eee; } +.nav a { color: #111; text-decoration: none; } +.page { padding: 2rem; } +.hero-title { font-size: 2rem; margin: 0 0 0.5rem; } +.hero-hook { color: #555; } diff --git a/tests/framework-fixtures/tanstack-router-vite/files/vite.config.js b/tests/framework-fixtures/tanstack-router-vite/files/vite.config.js new file mode 100644 index 000000000..f7ad4b565 --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/files/vite.config.js @@ -0,0 +1,7 @@ +import { defineConfig } from 'vite'; +import react from '@vitejs/plugin-react'; + +export default defineConfig({ + plugins: [react()], + server: { host: '127.0.0.1', strictPort: false }, +}); diff --git a/tests/framework-fixtures/tanstack-router-vite/fixture.json b/tests/framework-fixtures/tanstack-router-vite/fixture.json new file mode 100644 index 000000000..e5f96bd86 --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/fixture.json @@ -0,0 +1,49 @@ +{ + "name": "Vite 8 + TanStack Router (code-based SPA)", + "config": { + "files": ["index.html"], + "insertBefore": "", + "commentSyntax": "html" + }, + "sourceFiles": [ + "index.html", + "src/main.jsx", + "src/routes/Home.jsx", + "src/routes/About.jsx", + "src/styles.css", + "vite.config.js" + ], + "generatedFiles": [], + "wrapCases": [ + { + "name": "wraps About hero in routes/About.jsx", + "args": { "classes": "hero-title", "tag": "h1" }, + "expectedFile": "src/routes/About.jsx" + } + ], + "runtime": { + "styling": "plain-css", + "install": ["npm", "install", "--no-audit", "--no-fund", "--loglevel=error"], + "devCommand": ["npx", "vite", "--host", "127.0.0.1"], + "readyPattern": "Local:\\s+https?://[^:]+:(\\d+)", + "readyTimeoutMs": 120000, + "preActions": [ + { "type": "click", "selector": "[data-testid='nav-about']" }, + { "type": "wait", "selector": "h1.hero-title" } + ], + "reloadProbe": { + "preActions": [ + { "type": "click", "selector": "[data-testid='nav-about']" }, + { "type": "wait", "selector": "h1.hero-title" } + ], + "expectSelector": "h1.hero-title" + }, + "probe": { + "expectLiveInit": true, + "expectConsoleClean": true + }, + "steer": { + "sourceFile": "src/routes/About.jsx" + } + } +} diff --git a/tests/framework-fixtures/tanstack-router-vite/gitignore.txt b/tests/framework-fixtures/tanstack-router-vite/gitignore.txt new file mode 100644 index 000000000..8cda9ad20 --- /dev/null +++ b/tests/framework-fixtures/tanstack-router-vite/gitignore.txt @@ -0,0 +1,4 @@ +node_modules/ +dist/ +.vite/ +package-lock.json diff --git a/tests/framework-fixtures/tanstack-start/files/package.json b/tests/framework-fixtures/tanstack-start/files/package.json new file mode 100644 index 000000000..83f93d047 --- /dev/null +++ b/tests/framework-fixtures/tanstack-start/files/package.json @@ -0,0 +1,21 @@ +{ + "name": "tanstack-start-fixture", + "private": true, + "version": "0.0.0", + "type": "module", + "scripts": { + "dev": "vite dev --host 127.0.0.1", + "build": "vite build", + "start": "node .output/server/index.mjs" + }, + "dependencies": { + "@tanstack/react-router": "^1.132.0", + "@tanstack/react-start": "^1.132.0", + "react": "^19.2.0", + "react-dom": "^19.2.0" + }, + "devDependencies": { + "@vitejs/plugin-react": "^6.0.0", + "vite": "^8.0.0" + } +} diff --git a/tests/framework-fixtures/tanstack-start/files/src/router.tsx b/tests/framework-fixtures/tanstack-start/files/src/router.tsx new file mode 100644 index 000000000..9cd826476 --- /dev/null +++ b/tests/framework-fixtures/tanstack-start/files/src/router.tsx @@ -0,0 +1,12 @@ +import { createRouter as createTanStackRouter } from '@tanstack/react-router'; +import { routeTree } from './routeTree.gen'; + +export function getRouter() { + return createTanStackRouter({ routeTree, scrollRestoration: true }); +} + +declare module '@tanstack/react-router' { + interface Register { + router: ReturnType; + } +} diff --git a/tests/framework-fixtures/tanstack-start/files/src/routes/__root.tsx b/tests/framework-fixtures/tanstack-start/files/src/routes/__root.tsx new file mode 100644 index 000000000..c3c0f0413 --- /dev/null +++ b/tests/framework-fixtures/tanstack-start/files/src/routes/__root.tsx @@ -0,0 +1,26 @@ +import { HeadContent, Scripts, createRootRoute } from '@tanstack/react-router'; + +export const Route = createRootRoute({ + head: () => ({ + meta: [ + { charSet: 'utf-8' }, + { name: 'viewport', content: 'width=device-width, initial-scale=1' }, + { title: 'TanStack Start Fixture' }, + ], + }), + shellComponent: RootDocument, +}); + +function RootDocument({ children }: { children: React.ReactNode }) { + return ( + + + + + + {children} + + + + ); +} diff --git a/tests/framework-fixtures/tanstack-start/files/src/routes/index.tsx b/tests/framework-fixtures/tanstack-start/files/src/routes/index.tsx new file mode 100644 index 000000000..52a49652f --- /dev/null +++ b/tests/framework-fixtures/tanstack-start/files/src/routes/index.tsx @@ -0,0 +1,14 @@ +import { createFileRoute } from '@tanstack/react-router'; + +export const Route = createFileRoute('/')({ + component: Home, +}); + +function Home() { + return ( +
+

Start Home Hero

+

Server-rendered by TanStack Start.

+
+ ); +} diff --git a/tests/framework-fixtures/tanstack-start/files/vite.config.js b/tests/framework-fixtures/tanstack-start/files/vite.config.js new file mode 100644 index 000000000..6479cd32e --- /dev/null +++ b/tests/framework-fixtures/tanstack-start/files/vite.config.js @@ -0,0 +1,8 @@ +import { defineConfig } from 'vite'; +import { tanstackStart } from '@tanstack/react-start/plugin/vite'; +import viteReact from '@vitejs/plugin-react'; + +export default defineConfig({ + server: { host: '127.0.0.1' }, + plugins: [tanstackStart(), viteReact()], +}); diff --git a/tests/framework-fixtures/tanstack-start/fixture.json b/tests/framework-fixtures/tanstack-start/fixture.json new file mode 100644 index 000000000..ca001aa30 --- /dev/null +++ b/tests/framework-fixtures/tanstack-start/fixture.json @@ -0,0 +1,40 @@ +{ + "name": "Vite 8 + TanStack Start (SSR, root-document adapter)", + "config": { + "files": ["src/routes/__root.tsx"], + "insertBefore": "=${ENGINES_NODE_MAJOR}?0:1)`; + function expectCommand(command, expectedPath) { assert.equal(typeof command, 'string'); - assert.match(command, /^node "/); + // node-command providers carry the missing-file guard (issue #399: exits 0 + // when absent, preserves node's exit code when present) plus the runtime + // probe. GitHub's portable `$(git rev-parse)` form is guarded too, so it + // lands in the same branch. + if (command.startsWith('[ ! -f "')) { + assert.match(command, /\|\| node "/); + assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`); + } else { + assert.match(command, /^node "|^bash -c|\$\(git rev-parse/); + } assert.ok(command.includes(expectedPath), `missing ${expectedPath} in ${command}`); assert.ok(!command.includes('hook-probe.mjs'), `probe hook still referenced in ${command}`); } +function manifestCommands(manifest) { + const commands = []; + const walk = (value) => { + if (Array.isArray(value)) { value.forEach(walk); return; } + if (value && typeof value === 'object') { + if (typeof value.command === 'string') commands.push(value.command); + if (typeof value.bash === 'string') commands.push(value.bash); + Object.values(value).forEach(walk); + } + }; + walk(manifest.hooks); + return commands; +} + describe('hook manifest builders', () => { it('builds Claude project settings for the real detector hook', () => { const manifest = buildClaudeSettingsManifest(); @@ -56,6 +90,9 @@ describe('hook manifest builders', () => { }); it('builds Codex project-local hooks for the real detector hook', () => { + // Default install dir is `.codex`: a `.codex`-directory install keeps the + // skill payload at `.codex/skills/...`, so the hook must point there (not at + // a hardcoded `.agents`, which no-ops on such installs). const manifest = buildCodexHooksManifest(); assert.equal(manifest.description, undefined); const group = manifest.hooks.PostToolUse[0]; @@ -65,7 +102,7 @@ describe('hook manifest builders', () => { assert.equal(handler.type, 'command'); assert.equal(handler.timeout, 5); assert.equal(handler.statusMessage, 'Checking UI changes'); - expectCommand(handler.command, '.agents/skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, '.codex/skills/impeccable/scripts/hook.mjs'); assert.ok(!handler.command.includes('git rev-parse --show-toplevel')); assert.ok(!handler.command.includes('${PLUGIN_ROOT}')); assert.equal(manifest.hooks.SessionStart, undefined); @@ -74,7 +111,31 @@ describe('hook manifest builders', () => { // pass too. const stop = manifest.hooks.Stop[0].hooks[0]; assert.equal(stop.timeout, 30); - expectCommand(stop.command, '.agents/skills/impeccable/scripts/hook.mjs'); + expectCommand(stop.command, '.codex/skills/impeccable/scripts/hook.mjs'); + }); + + it('derives the Codex hook payload path from the install dir', () => { + // Each install dir gets a manifest pointing at its own skills payload: a + // `.codex`-directory install at `.codex/skills`, a `.agents` (Codex repo + // skills) install at `.agents/skills`. + const codexDir = buildCodexHooksManifest('.codex'); + expectCommand(codexDir.hooks.PostToolUse[0].hooks[0].command, '.codex/skills/impeccable/scripts/hook.mjs'); + expectCommand(codexDir.hooks.Stop[0].hooks[0].command, '.codex/skills/impeccable/scripts/hook.mjs'); + + const agentsDir = buildCodexHooksManifest('.agents'); + expectCommand(agentsDir.hooks.PostToolUse[0].hooks[0].command, '.agents/skills/impeccable/scripts/hook.mjs'); + expectCommand(agentsDir.hooks.Stop[0].hooks[0].command, '.agents/skills/impeccable/scripts/hook.mjs'); + assert.ok(!agentsDir.hooks.PostToolUse[0].hooks[0].command.includes('.codex/skills')); + + // hooksJsonFor threads the provider's configDir through to the builder. + expectCommand( + hooksJsonFor('codex', { configDir: '.agents' }).hooks.PostToolUse[0].hooks[0].command, + '.agents/skills/impeccable/scripts/hook.mjs', + ); + expectCommand( + hooksJsonFor('codex').hooks.PostToolUse[0].hooks[0].command, + '.codex/skills/impeccable/scripts/hook.mjs', + ); }); it('builds one Cursor pre-write blocking hook', () => { @@ -131,6 +192,40 @@ describe('hook manifest builders', () => { expectCommand(stop.command, '.grok/skills/impeccable/scripts/hook.mjs'); }); + it('probes the node runtime everywhere, and notices only where a channel exists', () => { + // Claude Code and Codex render a `systemMessage` from hook stdout, so their + // manifests carry the one-time unsupported-runtime notice. Cursor (output is + // permission-shaped; a message would block the edit), Grok (stdout ignored), + // and Copilot (contract unconfirmed) get the silent probe only. + const withNotice = [ + buildClaudeSettingsManifest(), + buildClaudePluginHooksManifest(), + buildCodexHooksManifest(), + buildCodexPluginHooksManifest(), + ]; + const probeOnly = [ + buildCursorHooksManifest(), + buildGitHubHooksManifest(), + buildGrokHooksManifest(), + ]; + for (const manifest of [...withNotice, ...probeOnly]) { + for (const command of manifestCommands(manifest)) { + assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`); + } + } + for (const manifest of withNotice) { + for (const command of manifestCommands(manifest)) { + assert.ok(command.includes('systemMessage'), `missing notice in ${command}`); + assert.ok(command.includes('node-unsupported'), `missing once-only marker in ${command}`); + } + } + for (const manifest of probeOnly) { + for (const command of manifestCommands(manifest)) { + assert.ok(!command.includes('systemMessage'), `unexpected notice in ${command}`); + } + } + }); + it('routes supported hook builders and leaves other providers alone', () => { assert.ok(hooksJsonFor('claude')); assert.ok(hooksJsonFor('codex')); @@ -185,11 +280,25 @@ describe('generated hook artifacts in repo', () => { assert.ok(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/detector/detect-antipatterns.mjs'))); }); - it('Codex project hooks reference hook.mjs in the .agents skill payload', () => { + it('Codex project hooks reference hook.mjs in the .codex skill payload', () => { + // The committed `.codex/hooks.json` is the distribution artifact for a + // `.codex`-directory install, whose skill payload lives at `.codex/skills/` + // (issue: it previously hardcoded `.agents/skills`, so the guarded hook + // no-opped on `.codex` installs). CLI installs that lay the skill down at + // `.agents/skills` rewrite the command to that path at install time. const manifest = readJson('.codex/hooks.json'); const handler = manifest.hooks.PostToolUse[0].hooks[0]; - expectCommand(handler.command, '.agents/skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, '.codex/skills/impeccable/scripts/hook.mjs'); + assert.ok(!handler.command.includes('.agents/skills')); + + // The self-consistent Codex bundle at `dist/codex/.codex/skills/` is a build + // artifact, not a tracked repo file; `bun run build` emits it and + // build.test.js verifies it there. This suite runs before the build (CI's + // `test:core` precedes the Build step), so it asserts only tracked outputs. + + // The repo ships the Codex skill payload at `.agents/skills` (the + // layout CLI installs use, and where the rewritten command resolves). assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/SKILL.md'))); assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/scripts/hook.mjs'))); assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/scripts/hook-lib.mjs'))); diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index c50074a84..88db7bac5 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -53,6 +53,8 @@ import { IMMEDIATE_TIER_RULES, splitFindingsByTier, perEditTieringActive, + ADVISORY_RULES, + isAdvisoryFinding, payload, extractFindingIgnoreValue, resolveProjectPlatform, @@ -416,6 +418,39 @@ describe('filterFindings()', () => { assert.deepEqual(filtered.map((f) => f.antipattern), ['gradient-text', 'overused-font']); }); + it('drops advisory-rule findings by default', () => { + const findings = [ + finding('side-tab', 1), + finding('em-dash-overuse', 2), + finding('gradient-text', 3), + ]; + const filtered = filterFindings(findings, '', '.html', { + ignoreRules: [], + limits: DEFAULT_CONFIG.limits, + }); + assert.deepEqual(filtered.map((f) => f.antipattern), ['side-tab', 'gradient-text']); + }); + + it('keeps advisory-rule findings when advisoryRules is "include"', () => { + const findings = [ + finding('side-tab', 1), + finding('em-dash-overuse', 2), + ]; + const filtered = filterFindings(findings, '', '.html', { + ignoreRules: [], + advisoryRules: 'include', + limits: DEFAULT_CONFIG.limits, + }); + assert.deepEqual(filtered.map((f) => f.antipattern), ['side-tab', 'em-dash-overuse']); + }); + + it('recognizes advisory findings by rule id or explicit flag', () => { + assert.ok(ADVISORY_RULES.has('em-dash-overuse')); + assert.equal(isAdvisoryFinding(finding('em-dash-overuse', 1)), true); + assert.equal(isAdvisoryFinding({ antipattern: 'anything', advisory: true }), true); + assert.equal(isAdvisoryFinding(finding('side-tab', 1)), false); + }); + it('does not treat source comments as hook suppression', () => { const content = [ '/* impeccable: ignore * */', @@ -3147,12 +3182,12 @@ describe('runHook() — per-edit tiering', () => { it('splitFindingsByTier partitions on IMMEDIATE_TIER_RULES', () => { const { immediate, deferred } = splitFindingsByTier([ finding('dark-glow', 1), - finding('em-dash-overuse', 2), + finding('marketing-buzzword', 2), finding('low-contrast', 3), finding('side-tab', 4), ]); assert.deepEqual(immediate.map((f) => f.antipattern), ['dark-glow', 'low-contrast']); - assert.deepEqual(deferred.map((f) => f.antipattern), ['em-dash-overuse', 'side-tab']); + assert.deepEqual(deferred.map((f) => f.antipattern), ['marketing-buzzword', 'side-tab']); for (const f of immediate) assert.ok(IMMEDIATE_TIER_RULES.has(f.antipattern)); }); @@ -3167,13 +3202,13 @@ describe('runHook() — per-edit tiering', () => { it('surfaces immediate-tier findings per edit and defers copy-tier ones', async () => { const file = write('src/Card.tsx', 'noop'); const det = fakeDetector([ - finding('em-dash-overuse', 3), + finding('marketing-buzzword', 3), finding('dark-glow', 5), ]); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd, detector: det }); assert.match(r.stdout, /Design hook findings requiring review/); assert.match(r.stdout, /dark-glow/); - assert.doesNotMatch(r.stdout, /em-dash-overuse/); + assert.doesNotMatch(r.stdout, /marketing-buzzword/); assert.equal(r.audit.deferred, 1); const cache = readCache(cwd); @@ -3182,10 +3217,10 @@ describe('runHook() — per-edit tiering', () => { it('emits a clean ack when all findings are deferred, and still marks the file touched', async () => { const file = write('src/Copy.tsx', 'noop'); - const det = fakeDetector([finding('em-dash-overuse', 2)]); + const det = fakeDetector([finding('marketing-buzzword', 2)]); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'tier-deferred-only')), env: {}, cwd, detector: det }); assert.match(r.stdout, /No deterministic design-quality issues found/); - assert.doesNotMatch(r.stdout, /em-dash-overuse/); + assert.doesNotMatch(r.stdout, /marketing-buzzword/); assert.equal(r.audit.deferred, 1); // The touched-file entry is what lets the Stop deep pass find this file. @@ -3198,10 +3233,10 @@ describe('runHook() — per-edit tiering', () => { fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ hook: { perEditRules: 'all' } })); const file = write('src/Card.tsx', 'noop'); - const det = fakeDetector([finding('em-dash-overuse', 2)]); + const det = fakeDetector([finding('marketing-buzzword', 2)]); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'tier-all')), env: {}, cwd, detector: det }); assert.match(r.stdout, /Design hook findings requiring review/); - assert.match(r.stdout, /em-dash-overuse/); + assert.match(r.stdout, /marketing-buzzword/); assert.equal(r.audit.deferred, undefined); }); @@ -3213,11 +3248,33 @@ describe('runHook() — per-edit tiering', () => { toolName: 'edit', toolArgs: JSON.stringify({ path: file }), }; - const det = fakeDetector([finding('em-dash-overuse', 2)]); + const det = fakeDetector([finding('marketing-buzzword', 2)]); const r = await runHook({ stdinJson: JSON.stringify(githubEvent), env: {}, cwd, detector: det }); assert.equal(r.audit.harness, 'github'); const out = JSON.parse(r.stdout); - assert.match(out.additionalContext, /em-dash-overuse/); + assert.match(out.additionalContext, /marketing-buzzword/); + }); + + it('skips advisory findings per edit by default and never nags about them', async () => { + const file = write('src/Copy.tsx', 'noop'); + const det = fakeDetector([finding('em-dash-overuse', 3)]); + const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'adv-skip')), env: {}, cwd, detector: det }); + // The only finding is advisory, so the file scans clean. + assert.match(r.stdout, /No deterministic design-quality issues found/); + assert.doesNotMatch(r.stdout, /em-dash-overuse/); + }); + + it('includes advisory findings per edit when detector.advisoryRules is "include"', async () => { + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ + hook: { perEditRules: 'all' }, + detector: { advisoryRules: 'include' }, + })); + const file = write('src/Copy.tsx', 'noop'); + const det = fakeDetector([finding('em-dash-overuse', 3)]); + const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'adv-include')), env: {}, cwd, detector: det }); + assert.match(r.stdout, /Design hook findings requiring review/); + assert.match(r.stdout, /em-dash-overuse/); }); }); @@ -3257,14 +3314,14 @@ describe('runStopHook()', () => { const file = write('src/Card.tsx', 'noop'); const det = fakeDetector([ finding('dark-glow', 5), - finding('em-dash-overuse', 3), + finding('marketing-buzzword', 3), finding('side-tab', 7), ]); // Per-edit pass: surfaces dark-glow, defers the other two. const edit = await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); assert.match(edit.stdout, /dark-glow/); - assert.doesNotMatch(edit.stdout, /em-dash-overuse/); + assert.doesNotMatch(edit.stdout, /marketing-buzzword/); // Stop deep pass: surfaces exactly the deferred remainder. const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); @@ -3272,7 +3329,7 @@ describe('runStopHook()', () => { assert.equal(stop.audit.emitted, true); const out = JSON.parse(stop.stdout); assert.equal(out.hookSpecificOutput.hookEventName, 'Stop'); - assert.match(out.hookSpecificOutput.additionalContext, /em-dash-overuse/); + assert.match(out.hookSpecificOutput.additionalContext, /marketing-buzzword/); assert.match(out.hookSpecificOutput.additionalContext, /side-tab/); assert.doesNotMatch(out.hookSpecificOutput.additionalContext, /dark-glow/); assert.equal(stop.emission.kind, 'stop-deep-pass'); @@ -3288,11 +3345,11 @@ describe('runStopHook()', () => { it('a second Stop fire is silent: deep-pass findings are remembered', async () => { const sid = 'stop-twice'; const file = write('src/Card.tsx', 'noop'); - const det = fakeDetector([finding('em-dash-overuse', 3)]); + const det = fakeDetector([finding('marketing-buzzword', 3)]); await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); const first = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); - assert.match(first.stdout, /em-dash-overuse/); + assert.match(first.stdout, /marketing-buzzword/); const second = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); assert.equal(second.stdout, ''); @@ -3303,15 +3360,84 @@ describe('runStopHook()', () => { const sid = 'stop-ignored'; fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ - detector: { ignoreRules: ['em-dash-overuse'] }, + detector: { ignoreRules: ['marketing-buzzword'] }, + })); + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('marketing-buzzword', 3)]); + + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(stop.stdout, ''); + assert.equal(stop.audit.skipped, 'stop-clean'); + }); + + it('skips advisory findings in the deep pass by default', async () => { + const sid = 'stop-advisory'; + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('em-dash-overuse', 3)]); + + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + // Silent either way: the advisory finding is dropped at the per-edit pass, so + // the file is never recorded as touched, and the deep pass has nothing to say. + assert.equal(stop.stdout, ''); + assert.ok(['stop-clean', 'no-touched-files'].includes(stop.audit.skipped)); + }); + + it('surfaces advisory findings in the deep pass when advisoryRules is "include"', async () => { + const sid = 'stop-advisory-include'; + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ + detector: { advisoryRules: 'include' }, })); const file = write('src/Card.tsx', 'noop'); const det = fakeDetector([finding('em-dash-overuse', 3)]); await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(stop.audit.emitted, true); + const out = JSON.parse(stop.stdout); + assert.match(out.hookSpecificOutput.additionalContext, /em-dash-overuse/); + }); + + it('re-invoked with stop_hook_active:true exits 0 and silent even with pending findings (issue #400)', async () => { + const sid = 'stop-active'; + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('marketing-buzzword', 3)]); + + // Prime a real touched-file + finding so a plain Stop pass would fire. + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + + // Re-invocation after the previous fire kept the turn alive: the contract + // says exit clean, no re-block, before scanning. + const active = { ...stopEvent(sid), stop_hook_active: true }; + const stop = await runStopHook({ stdinJson: JSON.stringify(active), env: {}, cwd, detector: det }); + assert.equal(stop.exitCode, 0); assert.equal(stop.stdout, ''); - assert.equal(stop.audit.skipped, 'stop-clean'); + assert.equal(stop.audit.emitted, undefined); + assert.equal(stop.audit.skipped, 'stop-hook-active'); + }); + + it('stop_hook_active:false or absent still runs the deep pass as before', async () => { + const sid = 'stop-inactive'; + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('marketing-buzzword', 3)]); + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + + // Explicit false (the stopEvent default). + const explicitFalse = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(explicitFalse.audit.emitted, true); + assert.match(explicitFalse.stdout, /marketing-buzzword/); + + // Field absent entirely (legacy / non-Claude-Code payloads): same behavior. + const sid2 = 'stop-absent'; + const file2 = write('src/Card2.tsx', 'noop'); + await runHook({ stdinJson: JSON.stringify(editEvent(file2, sid2)), env: {}, cwd, detector: det }); + const ev = stopEvent(sid2); + delete ev.stop_hook_active; + const absent = await runStopHook({ stdinJson: JSON.stringify(ev), env: {}, cwd, detector: det }); + assert.equal(absent.audit.emitted, true); + assert.match(absent.stdout, /marketing-buzzword/); }); it('honors kill switches and the re-entrancy guard', async () => { diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs index e2066e664..2106bc1a6 100644 --- a/tests/live-browser-regression.test.mjs +++ b/tests/live-browser-regression.test.mjs @@ -273,6 +273,73 @@ describe('live-browser.js regression guards', () => { ); }); + it('SSE error reply clears the durable session checkpoint like discarded', () => { + // Issue #362: `live-poll.mjs --reply error "..."` is the documented + // abort flow in reference/live.md, and an agent error reply is terminal + // for the session it names. The 'error' case used to reset only the UI + // (hideBar + PICKING) while the localStorage checkpoint written for the + // GENERATING phase survived — so every reload resurrected a dead session + // the server no longer knew about, until the user hand-cleared the + // impeccable-live* keys in the console. The error path must tear down the + // named session exactly like 'discarded' does (markSessionHandled + + // cleanup, which includes clearSession), and drop a stored-but-not- + // current checkpoint that matches the errored id. + const errorCase = SOURCE.match(/case 'error':[\s\S]{0,2500}?setLiveState\('PICKING'\);\s*break;/); + assert.ok(errorCase, 'expected an SSE case \'error\' handler in live-browser.js'); + assert.match( + errorCase[0], + /if \(msg\.id && msg\.id === currentSessionId\) \{[\s\S]{0,160}?markSessionHandled\(\);[\s\S]{0,80}?cleanup\(\);[\s\S]{0,80}?break;/, + 'an error reply naming the current session must run the same markSessionHandled + cleanup teardown as \'discarded\' so the durable checkpoint is cleared', + ); + assert.match( + errorCase[0], + /if \(msg\.id && loadSession\(\)\?\.id === msg\.id\) clearSession\(\);/, + 'an error reply naming a stored-but-not-current session must drop that checkpoint so a reload cannot resurrect it', + ); + }); + + it('a late accept failure is recognized after the optimistic teardown (#384)', () => { + // Accept is optimistic: POST /events acknowledging the intent schedules + // cleanupAcceptedSession(), which nulls pendingAcceptedSession before + // live-accept.mjs has actually run. When the accept later fails (missing + // markers, preview error, receipt conflict, source_locked), the SSE + // 'error' guard keyed on pendingAcceptedSession could no longer match, + // so the user got only the generic error toast with no hint that their + // variant was never written. An awaitingAcceptResult id must be set on + // the optimistic success path, survive cleanupAcceptedSession, be + // matched in the 'error' case with an explicit not-saved message, and + // be released when the real accept result arrives. + assert.match( + SOURCE, + /awaitingAcceptResult = \{ id: acceptedSessionId \};[\s\S]{0,400}?scheduleAcceptCleanup\(pending\);/, + 'the optimistic POST-success path must record awaitingAcceptResult before scheduling the teardown', + ); + const errorCase = SOURCE.match(/case 'error':[\s\S]{0,2600}?setLiveState\('PICKING'\);\s*break;/); + assert.ok(errorCase, 'expected an SSE case \'error\' handler in live-browser.js'); + assert.match( + errorCase[0], + /if \(awaitingAcceptResult\?\.id && msg\.id === awaitingAcceptResult\.id\) \{[\s\S]{0,700}?awaitingAcceptResult = null;[\s\S]{0,700}?may not have been saved[\s\S]{0,300}?break;/, + 'an error naming the awaited accept must clear the marker and warn that the variant may not have been saved (hedged: a carbonize-phase failure fires this after the source WAS promoted)', + ); + // Accept unlocks at the first variant, so a late generation agent_done + // for the same session id can arrive after Accept; only a carbonize + // agent_done is provably accept-side and may close the window. + const agentDoneCase = SOURCE.match(/case 'agent_done':[\s\S]{0,1200}?break;/); + assert.ok(agentDoneCase, 'expected an SSE case \'agent_done\' handler in live-browser.js'); + assert.match( + agentDoneCase[0], + /msg\.data\?\.carbonize === true && awaitingAcceptResult\?\.id && msg\.id === awaitingAcceptResult\.id/, + 'agent_done must only release the awaited accept marker for carbonize completions, or a late generation agent_done reopens the #384 hole', + ); + const cleanupFn = SOURCE.match(/function cleanupAcceptedSession\(\) \{[\s\S]{0,1200}?\n \}/); + assert.ok(cleanupFn, 'expected cleanupAcceptedSession in live-browser.js'); + assert.doesNotMatch( + cleanupFn[0], + /awaitingAcceptResult\s*=/, + 'cleanupAcceptedSession must not clear awaitingAcceptResult - surviving the teardown is the point', + ); + }); + it('handleServerLost preserves the current recoverable phase', () => { assert.doesNotMatch( SOURCE, @@ -286,6 +353,41 @@ describe('live-browser.js regression guards', () => { ); }); + it('server-lost toast frames the disconnect as resumable, not ended', () => { + assert.doesNotMatch( + SOURCE, + /Live server disconnected\. Session ended\./, + 'the "Session ended" copy made agents rationalize bailing to direct edits; the session is resumable', + ); + assert.match( + SOURCE, + /Live server connection lost\. Your session is saved;[^']*restart live-poll\.mjs to continue\./, + 'server-lost toast should tell the user the session is saved and how to continue', + ); + }); + + it('the agent-phase progress bar advances monotonically', () => { + // A behind/resumed checkpoint must not move the visible bar backward. + assert.doesNotMatch( + SOURCE, + /generationPhase = msg\.phase \|\| generationPhase;/, + 'raw phase assignment lets a behind checkpoint regress the visible bar to an earlier phase', + ); + assert.match( + SOURCE, + /case 'agent_phase':[\s\S]{0,400}?if \(shouldAdvancePhase\(generationPhase, msg\.phase\)\) generationPhase = msg\.phase;/, + 'agent_phase should only advance the phase when it moves forward', + ); + // The rank table must order the lifecycle so scaffolding/source_ready sit + // below generating and the reviewable phases. + assert.match(SOURCE, /function shouldAdvancePhase\(current, next\)/); + assert.match( + SOURCE, + /scaffolding: 2,[\s\S]{0,120}?source_ready: 4,[\s\S]{0,120}?(generation_ready|generating): 5,/, + 'scaffolding and source_ready must rank below generating', + ); + }); + it('source reinjection preserves the visible variant after cycling', () => { assert.doesNotMatch( SOURCE, @@ -920,10 +1022,13 @@ describe('live-browser.js regression guards', () => { ); assert.match(SOURCE, /tune\.disabled = true/, 'pending Tune must be visibly loading but non-interactive'); assert.match(SOURCE, /Tune controls are ready\./, 'parameter arrival needs a clear ready indication'); + // Source-mode DOM injection is gated to the `done` branch (it races + // framework ownership mid-generation), but a params-only publication must + // still flip the Tune controls into their loading state on the checkpoint. assert.match( SOURCE, - /msg\.publicationKind !== 'params' && arrivedVariants >= targetArrived/, - 'a params-only publication must refresh even though the variant count is unchanged', + /case 'variant_progress':[\s\S]{0,120}?if \(msg\.publicationKind === 'params'\) parameterGenerationState = 'loading';/, + 'a params-only publication must mark Tune controls loading even though the variant count is unchanged', ); assert.match(SOURCE, /revisionDomain: 'browser'/, 'browser checkpoints must use their own revision domain'); }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index 86669e3a2..d2539df04 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -320,7 +320,7 @@ describe('live-browser source contracts', () => { ); assert.match( SOURCE, - /case 'complete':\s*case 'accept':\s*if \(maybeCompleteAcceptedSession\(msg\)\) break;/, + /case 'complete':\s*case 'accept':[\s\S]{0,400}?if \(maybeCompleteAcceptedSession\(msg\)\) break;/, 'final accepted DOM cleanup should be driven by explicit complete or harness accept replies', ); assert.match( @@ -340,8 +340,8 @@ describe('live-browser source contracts', () => { assert.match(agentDoneSource, /maybeCompleteAcceptedSession\(msg\)/); assert.match( SOURCE, - /function handleGo\(\)[\s\S]{0,900}?pendingAcceptedSession = null;[\s\S]{0,80}?currentSessionId = id8\(\);/, - 'starting a new generation should clear any stale accepted-session sentinel first', + /function handleGo\(\)[\s\S]{0,900}?pendingAcceptedSession = null;[\s\S]{0,400}?awaitingAcceptResult = null;[\s\S]{0,120}?currentSessionId = id8\(\);/, + 'starting a new generation should clear any stale accepted-session sentinel (and the awaited accept-result marker, #384) first', ); const handleAcceptStart = SOURCE.indexOf('function handleAccept()'); const maybeCompleteStart = SOURCE.indexOf('function maybeCompleteAcceptedSession', handleAcceptStart); @@ -421,11 +421,30 @@ describe('live-browser source contracts', () => { ); }); - it('loads progressive source checkpoints through the no-HMR fallback', () => { + it('does not source-inject per variant_progress checkpoint (HMR owns mid-generation reconciliation)', () => { + // Isolate the variant_progress handler body. + const progressCase = SOURCE.match(/case 'variant_progress':[\s\S]*?break;/); + assert.ok(progressCase, 'variant_progress case should exist'); + assert.doesNotMatch( + progressCase[0], + /injectVariantsFromSource\(/, + 'source-mode progress must not source-inject per checkpoint; it races React/Vue ownership and triggers removeChild errors', + ); + // The svelte-component progressive path stays. assert.match( - SOURCE, - /case 'variant_progress':[\s\S]{0,1400}?msg\.previewMode === 'source'[\s\S]{0,1000}?arrivedVariants >= targetArrived[\s\S]{0,260}?injectVariantsFromSource\(msg\.previewFile \|\| msg\.file, msg\.id\)/, - 'source-mode progress should let framework HMR settle before using the no-HMR fallback', + progressCase[0], + /injectSvelteComponentsFromManifest\(msg\.previewFile, msg\.id\)/, + 'component-preview progressive delivery must still stream per checkpoint', + ); + }); + + it('source-injects only on the final done branch, keeping the 750ms settle', () => { + const doneCase = SOURCE.match(/case 'done':[\s\S]*?break;\n {8}case /); + assert.ok(doneCase, 'done case should exist'); + assert.match( + doneCase[0], + /setTimeout\([\s\S]{0,260}?injectVariantsFromSource\(msg\.file, msg\.id, \{ generationCompleted: true \}\)[\s\S]{0,40}?\}, 750\)/, + 'done should source-inject via the 750ms fallback for harnesses without HMR', ); }); }); diff --git a/tests/live-e2e/agent.mjs b/tests/live-e2e/agent.mjs index 19aed9d53..7b6ea8c85 100644 --- a/tests/live-e2e/agent.mjs +++ b/tests/live-e2e/agent.mjs @@ -1292,14 +1292,12 @@ function renderVariantsBlock({ sessionId, indent, output, commentSyntax, file, s } /** - * Read the wrapped file, find the "insert below this line" marker, splice in - * the rendered variants block, write back. + * Splice the rendered variants block into an array of wrapper lines at the + * "insert below this line" marker. Pure: returns the new lines array. Used + * both against a whole file (wrapper already in source) and against a + * standalone wrapper block (deferred source write, agent writes it now). */ -async function spliceVariantsIntoWrapper({ tmp, wrapInfo, sessionId, output }) { - const filePath = path.join(tmp, wrapInfo.file); - const src = await fs.readFile(filePath, 'utf-8'); - const lines = src.split('\n'); - +function spliceVariantsIntoLines(lines, { sessionId, output, commentSyntax, file, styleMode }) { // Find the "Variants: insert below this line" comment line — definitive // marker, robust to any indentation off-by-one. Matches in any comment // style (HTML / JSX / Astro). @@ -1307,7 +1305,7 @@ async function spliceVariantsIntoWrapper({ tmp, wrapInfo, sessionId, output }) { l.includes('Variants: insert below this line'), ); if (markerIdx === -1) { - throw new Error('insert marker not found in ' + wrapInfo.file); + throw new Error('insert marker not found in ' + file); } const indent = (lines[markerIdx].match(/^\s*/) || [''])[0]; @@ -1320,26 +1318,73 @@ async function spliceVariantsIntoWrapper({ tmp, wrapInfo, sessionId, output }) { sessionId, indent: wrapperIndent, output, - commentSyntax: wrapInfo.commentSyntax, - file: wrapInfo.file, - styleMode: wrapInfo.styleMode, + commentSyntax, + file, + styleMode, }); const endMarkerIdx = lines.findIndex((line, index) => index > markerIdx && line.includes('impeccable-variants-end ' + sessionId), ); if (endMarkerIdx === -1) { - throw new Error('end marker not found in ' + wrapInfo.file); + throw new Error('end marker not found in ' + file); } - const tailIdx = wrapInfo.commentSyntax.open === '{/*' + const tailIdx = commentSyntax.open === '{/*' ? endMarkerIdx : endMarkerIdx - 1; - const next = [ + return [ ...lines.slice(0, markerIdx + 1), block, ...lines.slice(tailIdx), ]; +} + +/** + * Read the wrapped file, find the "insert below this line" marker, splice in + * the rendered variants block, write back. Used when the wrapper is already + * present in source (agent's own wrap fallback, no preflight). + */ +async function spliceVariantsIntoWrapper({ tmp, wrapInfo, sessionId, output }) { + const filePath = path.join(tmp, wrapInfo.file); + const src = await fs.readFile(filePath, 'utf-8'); + const lines = src.split('\n'); + const next = spliceVariantsIntoLines(lines, { + sessionId, + output, + commentSyntax: wrapInfo.commentSyntax, + file: wrapInfo.file, + styleMode: wrapInfo.styleMode, + }); + await fs.writeFile(filePath, next.join('\n'), 'utf-8'); +} + +/** + * Deferred source write (preflight computed the scaffold but left source + * untouched). Splice the variants into the scaffold's `wrapperBlock`, then + * replace the picked element's source range with the result in ONE write — + * the 3.5 atomic single-edit semantics. `replaceEndLine < replaceStartLine` + * expresses a pure insertion (insert mode). + */ +async function writeDeferredWrapperWithVariants({ tmp, wrapInfo, sessionId, output }) { + const filePath = path.join(tmp, wrapInfo.file); + const src = await fs.readFile(filePath, 'utf-8'); + const lines = src.split('\n'); + const wrapperLines = String(wrapInfo.wrapperBlock).split('\n'); + const splicedWrapper = spliceVariantsIntoLines(wrapperLines, { + sessionId, + output, + commentSyntax: wrapInfo.commentSyntax, + file: wrapInfo.file, + styleMode: wrapInfo.styleMode, + }); + const startIdx = wrapInfo.replaceStartLine - 1; + const endIdx = wrapInfo.replaceEndLine - 1; // may be startIdx-1 for insertion + const next = [ + ...lines.slice(0, startIdx), + ...splicedWrapper, + ...lines.slice(endIdx + 1), + ]; await fs.writeFile(filePath, next.join('\n'), 'utf-8'); } @@ -1661,6 +1706,8 @@ export async function runAgentLoop({ trace('agent.write.start', { id: event.id, file: wrapInfo.file }); if (wrapInfo.previewMode === 'svelte-component') { await writeSvelteComponentVariants({ tmp, wrapInfo, event, output, writeParams: true }); + } else if (wrapInfo.sourceWritten === false) { + await writeDeferredWrapperWithVariants({ tmp, wrapInfo, sessionId: event.id, output }); } else { await spliceVariantsIntoWrapper({ tmp, wrapInfo, sessionId: event.id, output }); } diff --git a/tests/live-e2e/session.mjs b/tests/live-e2e/session.mjs index 5bae56cdf..e091dcc86 100644 --- a/tests/live-e2e/session.mjs +++ b/tests/live-e2e/session.mjs @@ -117,10 +117,14 @@ export function stopLiveServer(tmp) { } catch { /* already gone */ } } -export function runInject(tmp, port) { +export function runInject(tmp, port, token) { const out = execFileSync( process.execPath, - [join(SCRIPTS_DIR, 'live-inject.mjs'), '--port', String(port)], + [ + join(SCRIPTS_DIR, 'live-inject.mjs'), + '--port', String(port), + ...(token ? ['--token', String(token)] : []), + ], { cwd: tmp, encoding: 'utf-8', @@ -296,7 +300,7 @@ export async function bootFixtureSession({ const injectStartedAt = Date.now(); trace('setup.inject.start', { fixture: name }); log(`live-inject --port ${live.port}`); - const injectResult = runInject(tmp, live.port); + const injectResult = runInject(tmp, live.port, live.token); if (!injectResult.ok) throw new Error('live-inject failed: ' + JSON.stringify(injectResult)); trace('setup.inject.end', { fixture: name, files: injectResult.files || injectResult.pageFiles || [] }); log(`live-inject complete in ${formatDuration(Date.now() - injectStartedAt)}`); diff --git a/tests/live-generation-preflight.test.mjs b/tests/live-generation-preflight.test.mjs index a14b97bb1..099645226 100644 --- a/tests/live-generation-preflight.test.mjs +++ b/tests/live-generation-preflight.test.mjs @@ -5,6 +5,7 @@ import path from 'node:path'; import { buildGenerationPreflight, runGenerationPreflight, + clearSourceResolutionCache, } from '../skill/scripts/live/generation-preflight.mjs'; const SCRIPTS_DIR = path.resolve('skill/scripts'); @@ -26,6 +27,7 @@ test('builds a replace preflight from the picker locator', () => { assert.equal(command.mode, 'replace'); assert.deepEqual(command.args.slice(1), [ '--id', 'session-1', '--count', '3', + '--defer-source-write', '--element-id', 'hero', '--classes', 'hero hero--dark', '--tag', 'SECTION', @@ -48,11 +50,22 @@ test('builds an insert preflight from the anchor locator', () => { assert.equal(command.mode, 'insert'); assert.deepEqual(command.args.slice(1), [ - '--id', 'session-2', '--count', '2', '--position', 'before', + '--id', 'session-2', '--count', '2', + '--defer-source-write', '--position', 'before', '--classes', 'card', '--tag', 'ARTICLE', '--text', 'Plan', ]); }); +test('replace preflight always requests a deferred source write', () => { + const command = buildGenerationPreflight({ + type: 'generate', + id: 'session-defer', + count: 3, + element: { classes: ['hero'] }, + }, SCRIPTS_DIR); + assert.ok(command.args.includes('--defer-source-write')); +}); + test('returns scaffold metadata without exposing child-process details', async () => { const calls = []; const result = await runGenerationPreflight({ @@ -108,6 +121,93 @@ test('yields to the event loop instead of blocking on the child process', async assert.equal(tickedDuringPreflight, true, 'the event loop must stay responsive during preflight'); }); +test('caches the resolved source file and reuses it via --file on the next generate', async () => { + clearSourceResolutionCache(); + const cache = new Map(); + const event = { + type: 'generate', + id: 'sess-a', + count: 3, + pageUrl: '/pricing', + element: { classes: ['hero'], tagName: 'SECTION' }, + }; + const firstArgs = []; + const first = await runGenerationPreflight(event, { + scriptsDir: SCRIPTS_DIR, + cache, + async execFileImpl(_file, args) { + firstArgs.push(...args); + return { stdout: '{"file":"src/Pricing.jsx","sourceWritten":false}\n', stderr: '' }; + }, + }); + assert.equal(first.ok, true); + assert.ok(!firstArgs.includes('--file'), 'first pass does the tree search, no --file'); + + // Second generate on the SAME target (new session id) should point --file at + // the cached resolution and skip the search. + const secondArgs = []; + const second = await runGenerationPreflight({ ...event, id: 'sess-b' }, { + scriptsDir: SCRIPTS_DIR, + cache, + async execFileImpl(_file, args) { + secondArgs.push(...args); + return { stdout: '{"file":"src/Pricing.jsx","sourceWritten":false}\n', stderr: '' }; + }, + }); + assert.equal(second.ok, true); + const fileIdx = secondArgs.indexOf('--file'); + assert.notEqual(fileIdx, -1, 'cached resolution injects --file'); + assert.equal(secondArgs[fileIdx + 1], 'src/Pricing.jsx'); +}); + +test('evicts the cached resolution when the preflight fails', async () => { + const cache = new Map(); + const event = { + type: 'generate', + id: 'sess-c', + count: 3, + pageUrl: '/pricing', + element: { classes: ['hero'] }, + }; + await runGenerationPreflight(event, { + scriptsDir: SCRIPTS_DIR, + cache, + async execFileImpl() { return { stdout: '{"file":"src/Pricing.jsx"}\n', stderr: '' }; }, + }); + assert.equal(cache.size, 1); + + const error = new Error('spawn failed'); + error.stderr = 'live-wrap.mjs: element not found\n'; + await runGenerationPreflight(event, { + scriptsDir: SCRIPTS_DIR, + cache, + execFileImpl: () => Promise.reject(error), + }); + assert.equal(cache.size, 0, 'a failed resolution is evicted so the next run re-searches'); +}); + +test('caches the route source file, not the svelte-component manifest', async () => { + const cache = new Map(); + const event = { + type: 'generate', + id: 'sess-svelte', + count: 3, + pageUrl: '/', + element: { classes: ['hero'] }, + }; + await runGenerationPreflight(event, { + scriptsDir: SCRIPTS_DIR, + cache, + async execFileImpl() { + return { + stdout: '{"file":"node_modules/.impeccable-live/x/manifest.json","sourceFile":"src/routes/+page.svelte","previewMode":"svelte-component"}\n', + stderr: '', + }; + }, + }); + assert.deepEqual([...cache.values()], ['src/routes/+page.svelte']); +}); + test('reports a child-process failure without leaking internals or throwing', async () => { const error = new Error('spawn failed'); error.stderr = 'live-wrap.mjs: element not found\n'; diff --git a/tests/live-server.test.mjs b/tests/live-server.test.mjs index 29ba5adc2..dc8d7fa34 100644 --- a/tests/live-server.test.mjs +++ b/tests/live-server.test.mjs @@ -5,7 +5,7 @@ import { describe, it, before, after } from 'node:test'; import assert from 'node:assert/strict'; -import { existsSync, mkdtempSync, readFileSync, writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, writeFileSync, mkdirSync, rmSync, realpathSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { execFileSync, execSync, spawn } from 'node:child_process'; @@ -167,7 +167,7 @@ describe('live-server integration', () => { // rather than an inline copy, so the server must serialize the canonical // vocabulary into /live.js (next to the token/port). const { LIVE_COMMANDS } = await import('../skill/scripts/live/vocabulary.mjs'); - const body = await (await fetch(`http://localhost:${server.port}/live.js`)).text(); + const body = await (await fetch(`http://localhost:${server.port}/live.js?token=${server.token}`)).text(); assert.match(body, /window\.__IMPECCABLE_VOCAB__\s*=/); const injected = JSON.parse(body.match(/window\.__IMPECCABLE_VOCAB__\s*=\s*(\[.*?\]);/s)[1]); assert.deepEqual(injected, LIVE_COMMANDS); @@ -259,7 +259,7 @@ describe('live-server integration', () => { }); it('/live.js serves script with token injected', async () => { - const res = await fetch(`http://localhost:${server.port}/live.js`); + const res = await fetch(`http://localhost:${server.port}/live.js?token=${server.token}`); assert.equal(res.status, 200); assert.equal(res.headers.get('content-type'), 'application/javascript'); const text = await res.text(); @@ -302,6 +302,64 @@ describe('live-server integration', () => { ); }); + it('/live.js returns 401 without the token and 200 with it', async () => { + const noToken = await fetch(`http://localhost:${server.port}/live.js`); + assert.equal(noToken.status, 401); + + const wrongToken = await fetch(`http://localhost:${server.port}/live.js?token=not-the-token`); + assert.equal(wrongToken.status, 401); + + const ok = await fetch(`http://localhost:${server.port}/live.js?token=${server.token}`); + assert.equal(ok.status, 200); + const body = await ok.text(); + assert.ok(body.includes('__IMPECCABLE_LIVE_INIT__'), 'authorized /live.js returns the assembled bundle'); + }); + + it('CORS: a remote origin gets no Access-Control-Allow-Origin on any route', async () => { + const evil = 'https://evil.example'; + for (const path of ['/health', `/live.js?token=${server.token}`, `/status?token=${server.token}`]) { + const res = await fetch(`http://localhost:${server.port}${path}`, { headers: { Origin: evil } }); + assert.equal( + res.headers.get('access-control-allow-origin'), + null, + `remote origin must not be reflected on ${path}`, + ); + } + // Preflight from a remote origin is likewise unauthorized to read. + const preflight = await fetch(`http://localhost:${server.port}/poll`, { + method: 'OPTIONS', + headers: { Origin: evil, 'Access-Control-Request-Method': 'POST' }, + }); + assert.equal(preflight.headers.get('access-control-allow-origin'), null); + }); + + it('CORS: a loopback origin is reflected with Vary: Origin', async () => { + for (const origin of [ + `http://localhost:${server.port}`, + 'http://127.0.0.1:5173', + 'http://[::1]:5173', + ]) { + const res = await fetch(`http://localhost:${server.port}/health`, { headers: { Origin: origin } }); + assert.equal(res.headers.get('access-control-allow-origin'), origin, `reflect ${origin}`); + const vary = res.headers.get('vary') || ''; + assert.ok(/\bOrigin\b/i.test(vary), `Vary: Origin present for ${origin}, got "${vary}"`); + } + // A hostname that merely extends "localhost" must not pass the loopback test. + const spoof = await fetch(`http://localhost:${server.port}/health`, { + headers: { Origin: 'http://localhost.evil.com' }, + }); + assert.equal(spoof.headers.get('access-control-allow-origin'), null, 'localhost.evil.com must not be reflected'); + }); + + it('token-guarded routes still work with a loopback Origin header', async () => { + const origin = `http://localhost:${server.port}`; + const res = await fetch(`http://localhost:${server.port}/status?token=${server.token}`, { + headers: { Origin: origin }, + }); + assert.equal(res.status, 200); + assert.equal(res.headers.get('access-control-allow-origin'), origin); + }); + it('/design-system.json reads DESIGN.md plus .impeccable/design.json', async () => { const tmp = mkdtempSync(join(tmpdir(), 'impeccable-design-system-')); let designServer; @@ -3080,6 +3138,48 @@ colors: {} } }); + it('/source rejects an absolute path to a sibling directory sharing the root prefix', async () => { + // Sibling dir whose name extends the project dir name (projeto -> projeto-evil): + // a plain string prefix check on the resolved path lets it escape the root. + // Build it off the server's real cwd (macOS symlinks /var -> /private/var, + // and the server guards against its own process.cwd(), i.e. the realpath). + const siblingDir = realpathSync(serverCwd) + '-evil'; + mkdirSync(siblingDir, { recursive: true }); + const secretPath = join(siblingDir, 'secret.txt'); + writeFileSync(secretPath, 'TOP SECRET SIBLING'); + try { + const res = await fetch(`http://localhost:${server.port}/source?token=${server.token}&path=${encodeURIComponent(secretPath)}`); + // Drain the body so the socket doesn't hang regardless of status. + await res.text().catch(() => {}); + assert.equal(res.status, 403); + } finally { + rmSync(siblingDir, { recursive: true, force: true }); + } + }); + + it('/source rejects the project root itself (directory, not a file)', async () => { + // `.` resolves exactly to cwd; the route only serves files, so an empty + // relative path is not a legitimate request and must be forbidden. + const res = await fetch(`http://localhost:${server.port}/source?token=${server.token}&path=${encodeURIComponent('.')}`); + await res.text().catch(() => {}); + assert.equal(res.status, 403); + }); + + it('/source still serves a legitimate nested in-root file', async () => { + const nestedDir = join(serverCwd, 'nested'); + mkdirSync(nestedDir, { recursive: true }); + const nestedPath = join(nestedDir, 'page.html'); + writeFileSync(nestedPath, '

in root

\n'); + try { + const res = await fetch(`http://localhost:${server.port}/source?token=${server.token}&path=${encodeURIComponent('nested/page.html')}`); + assert.equal(res.status, 200); + const text = await res.text(); + assert.ok(text.includes('in root')); + } finally { + rmSync(nestedDir, { recursive: true, force: true }); + } + }); + it('/modern-screenshot.js serves the vendored UMD build', async () => { const res = await fetch(`http://localhost:${server.port}/modern-screenshot.js`); assert.equal(res.status, 200); diff --git a/tests/live-tanstack-adapter.test.mjs b/tests/live-tanstack-adapter.test.mjs new file mode 100644 index 000000000..141369ab3 --- /dev/null +++ b/tests/live-tanstack-adapter.test.mjs @@ -0,0 +1,175 @@ +/** + * Unit tests for the TanStack Start live-mode adapter. + * Run with: node --test tests/live-tanstack-adapter.test.mjs + */ + +import { describe, it, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { fileURLToPath } from 'node:url'; + +import { + detectTanStackStartProject, + applyTanStackLiveAdapter, + removeTanStackLiveAdapter, + patchTanStackRoot, + unpatchTanStackRoot, + buildTanStackLiveRootComponent, +} from '../skill/scripts/live/tanstack-adapter.mjs'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +const ROOT_TSX = `import { HeadContent, Scripts, createRootRoute } from '@tanstack/react-router'; + +export const Route = createRootRoute({ + shellComponent: RootDocument, +}); + +function RootDocument({ children }: { children: React.ReactNode }) { + return ( + + + + + + {children} + + + + ); +} +`; + +function scaffold(tmp, { ext = 'tsx', rootBody = ROOT_TSX, startPackage = '@tanstack/react-start' } = {}) { + mkdirSync(join(tmp, 'src', 'routes'), { recursive: true }); + writeFileSync(join(tmp, 'package.json'), JSON.stringify({ + name: 'app', + dependencies: { '@tanstack/react-router': '^1', [startPackage]: '^1' }, + })); + writeFileSync(join(tmp, 'src', 'routes', `__root.${ext}`), rootBody); +} + +describe('tanstack-adapter — detection', () => { + let tmp; + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-tanstack-')); }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + it('detects a TanStack Start project from package + root route', () => { + scaffold(tmp); + const project = detectTanStackStartProject(tmp); + assert.equal(project.rootRoute, 'src/routes/__root.tsx'); + assert.equal(project.componentFile, 'src/impeccable/ImpeccableLiveRoot.tsx'); + assert.equal(project.componentImport, '../impeccable/ImpeccableLiveRoot'); + }); + + it('mirrors the root-route extension for the mount component (jsx)', () => { + scaffold(tmp, { ext: 'jsx' }); + const project = detectTanStackStartProject(tmp); + assert.equal(project.rootRoute, 'src/routes/__root.jsx'); + assert.equal(project.componentFile, 'src/impeccable/ImpeccableLiveRoot.jsx'); + }); + + it('detects @tanstack/solid-start and @tanstack/start too', () => { + scaffold(tmp, { startPackage: '@tanstack/solid-start' }); + assert.ok(detectTanStackStartProject(tmp)); + }); + + it('returns null without the Start package (plain TanStack Router SPA)', () => { + mkdirSync(join(tmp, 'src', 'routes'), { recursive: true }); + writeFileSync(join(tmp, 'package.json'), JSON.stringify({ + dependencies: { '@tanstack/react-router': '^1' }, + })); + writeFileSync(join(tmp, 'src', 'routes', '__root.tsx'), ROOT_TSX); + assert.equal(detectTanStackStartProject(tmp), null); + }); + + it('returns null without a root route file', () => { + writeFileSync(join(tmp, 'package.json'), JSON.stringify({ + dependencies: { '@tanstack/react-start': '^1' }, + })); + assert.equal(detectTanStackStartProject(tmp), null); + }); +}); + +describe('tanstack-adapter — patch/unpatch round-trip', () => { + it('inserts the import + mount component before ', () => { + const patched = patchTanStackRoot(ROOT_TSX, '../impeccable/ImpeccableLiveRoot'); + assert.match(patched, /import ImpeccableLiveRoot from '\.\.\/impeccable\/ImpeccableLiveRoot';/); + assert.match(patched, /\{\/\* impeccable-live-tanstack-start \*\/\}/); + assert.match(patched, //); + // component renders before + assert.ok(patched.indexOf('') < patched.indexOf('')); + }); + + it('round-trips byte-for-byte (patch then unpatch)', () => { + const patched = patchTanStackRoot(ROOT_TSX, '../impeccable/ImpeccableLiveRoot'); + assert.notEqual(patched, ROOT_TSX); + assert.equal(unpatchTanStackRoot(patched), ROOT_TSX); + }); + + it('is idempotent (double patch adds one import + one mount)', () => { + const once = patchTanStackRoot(ROOT_TSX, '../impeccable/ImpeccableLiveRoot'); + const twice = patchTanStackRoot(once, '../impeccable/ImpeccableLiveRoot'); + assert.equal(twice, once); + assert.equal((twice.match(//g) || []).length, 1); + assert.equal((twice.match(/^import ImpeccableLiveRoot/gm) || []).length, 1); + }); + + it('falls back to when is absent', () => { + const noScripts = ROOT_TSX.replace(/\s*/, ''); + const patched = patchTanStackRoot(noScripts, '../impeccable/ImpeccableLiveRoot'); + assert.match(patched, //); + assert.ok(patched.indexOf('') < patched.indexOf('')); + assert.equal(unpatchTanStackRoot(patched), noScripts); + }); + + it('builds a client-only mount component carrying the token', () => { + const body = buildTanStackLiveRootComponent(8123, 'tok-xyz'); + assert.match(body, /http:\/\/localhost:8123\/live\.js\?token=tok-xyz/); + assert.match(body, /useEffect/); + assert.match(body, /typeof document === 'undefined'/); + assert.match(body, /data-impeccable-live-tanstack/); + }); +}); + +describe('tanstack-adapter — apply/remove on disk', () => { + let tmp; + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-tanstack-')); }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + it('apply writes the component + patches root, remove restores byte-for-byte', () => { + scaffold(tmp); + const original = readFileSync(join(tmp, 'src/routes/__root.tsx'), 'utf-8'); + + const applied = applyTanStackLiveAdapter({ cwd: tmp, port: 9100, token: 'T1' }); + assert.equal(applied.adapter, 'tanstack-start'); + assert.equal(applied.inserted, true); + assert.ok(existsSync(join(tmp, 'src/impeccable/ImpeccableLiveRoot.tsx'))); + assert.match(readFileSync(join(tmp, 'src/routes/__root.tsx'), 'utf-8'), /ImpeccableLiveRoot/); + assert.match( + readFileSync(join(tmp, 'src/impeccable/ImpeccableLiveRoot.tsx'), 'utf-8'), + /localhost:9100\/live\.js\?token=T1/, + ); + + const removed = removeTanStackLiveAdapter({ cwd: tmp }); + assert.equal(removed.removed, true); + assert.equal(existsSync(join(tmp, 'src/impeccable/ImpeccableLiveRoot.tsx')), false); + assert.equal(existsSync(join(tmp, 'src/impeccable')), false, 'empty managed dir pruned'); + assert.equal(readFileSync(join(tmp, 'src/routes/__root.tsx'), 'utf-8'), original); + }); + + it('refuses to clobber an unmanaged file at the component path', () => { + scaffold(tmp); + mkdirSync(join(tmp, 'src/impeccable'), { recursive: true }); + writeFileSync(join(tmp, 'src/impeccable/ImpeccableLiveRoot.tsx'), 'export const mine = 1;\n'); + const result = applyTanStackLiveAdapter({ cwd: tmp, port: 9100, token: 'T1' }); + assert.equal(result.error, 'tanstack_component_conflict'); + // unmanaged file untouched + assert.equal( + readFileSync(join(tmp, 'src/impeccable/ImpeccableLiveRoot.tsx'), 'utf-8'), + 'export const mine = 1;\n', + ); + }); +}); diff --git a/tests/live-wrap.test.mjs b/tests/live-wrap.test.mjs index b65d03d02..7140aa5dd 100644 --- a/tests/live-wrap.test.mjs +++ b/tests/live-wrap.test.mjs @@ -254,6 +254,43 @@ describe('wrapCli integration', () => { }); + it('--defer-source-write leaves source untouched and returns the wrapper block', () => { + const html = ` + + +
+

Hello World

+

Welcome to our site.

+
+ +`; + const file = join(tmp, 'index.html'); + writeFileSync(file, html); + + const result = JSON.parse(execSync( + `node skill/scripts/live-wrap.mjs --id defer1 --count 3 --classes "hero-section" --defer-source-write --file "${file}"`, + { cwd: process.cwd(), encoding: 'utf-8' } + )); + + // Source is NOT written by the preflight (no reload storm). + assert.equal(readFileSync(file, 'utf-8'), html); + + // Deferred contract fields present for the agent's atomic edit. + assert.equal(result.sourceWritten, false); + assert.ok(typeof result.wrapperBlock === 'string' && result.wrapperBlock.length > 0); + assert.ok(result.wrapperBlock.includes('data-impeccable-variants="defer1"')); + assert.ok(result.wrapperBlock.includes('Variants: insert below this line')); + assert.ok(result.wrapperBlock.includes('impeccable-variants-end defer1')); + assert.equal(typeof result.replaceStartLine, 'number'); + assert.equal(typeof result.replaceEndLine, 'number'); + + // The replace range points at the picked
block + // (1-indexed lines 4..7 of the source above). + const lines = html.split('\n'); + assert.ok(lines[result.replaceStartLine - 1].includes('class="hero-section"')); + assert.ok(lines[result.replaceEndLine - 1].includes('
')); + }); + it('wraps a JSX element and uses JSX comment syntax', () => { const jsx = `export default function App() { return ( diff --git a/tests/new-work-e2e.test.mjs b/tests/new-work-e2e.test.mjs new file mode 100644 index 000000000..0cbde6567 --- /dev/null +++ b/tests/new-work-e2e.test.mjs @@ -0,0 +1,357 @@ +/** + * Deterministic smoke tests for the new-work interactive flow. + * + * Covers the parts a user actually touches: the serve-question decision page + * (pick, re-roll + steer + re-deal, canon, tab close) driven through a real + * browser by the scripted user bot, plus the offline fake image generator. + * No LLM calls; a real Chromium via Playwright supplies full page fidelity + * (heartbeats, re-roll reload, tab close). Kept OUT of `bun run test` like + * live-e2e; run it with `bun run test:new-work-e2e`. + * + * The concept-seed direction roll (challengers, ASSIGNED INDEX, the no + * PRODUCT.md gate) is already covered by tests/concept-seed.test.mjs and is + * not repeated here. + * + * One-time setup: npx playwright install chromium + */ + +import { describe, it, before, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawn, spawnSync } from 'node:child_process'; +import { mkdtempSync, writeFileSync, mkdirSync, readFileSync, existsSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { runUserBot } from './new-work-e2e/user-bot.mjs'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const SERVE = path.join(ROOT, 'skill', 'scripts', 'serve-question.mjs'); +const GENERATE = path.join(ROOT, 'skill', 'scripts', 'generate-image.mjs'); +const CATALOG_DIR = path.join(ROOT, 'tests', 'fixtures', 'concept-catalog'); + +let playwright; +let browser; + +before(async () => { + try { + playwright = await import('playwright'); + } catch (err) { + throw new Error( + `Playwright is required for new-work-e2e tests (${err.message}). Run: npx playwright install chromium`, + ); + } + try { + browser = await playwright.chromium.launch({ headless: true }); + } catch (err) { + throw new Error(`Failed to launch Chromium (${err.message}). Run: npx playwright install chromium`); + } +}); + +after(async () => { + if (browser) await browser.close(); +}); + +// -------------------------------------------------------------------------- +// Workspace + serve-question helpers +// -------------------------------------------------------------------------- +function makeWorkspace() { + const dir = mkdtempSync(path.join(tmpdir(), 'new-work-e2e-')); + writeFileSync( + path.join(dir, 'PRODUCT.md'), + '# Product\n\n## Register\n\nbrand\n\n## Platform\n\nweb\n', + ); + return dir; +} + +// serve-question writes its state under cwd; run everything from the workspace. +function run(args, cwd) { + return new Promise((resolve) => { + const child = spawn(process.execPath, [SERVE, ...args], { + cwd, + env: { ...process.env, IMPECCABLE_QUESTION_FORCE: '1', IMPECCABLE_CATALOG_DIR: CATALOG_DIR }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let out = ''; + let err = ''; + child.stdout.on('data', (c) => { out += c; }); + child.stderr.on('data', (c) => { err += c; }); + child.on('exit', (code) => resolve({ code, out, err })); + }); +} + +async function startDaemon(cwd, payload, key) { + const payloadPath = path.join(cwd, `${key}.payload.json`); + writeFileSync(payloadPath, JSON.stringify(payload)); + const started = await run(['--start', '--payload', payloadPath, '--no-open', '--key', key], cwd); + assert.equal(started.code, 0, `--start failed: ${started.out} ${started.err}`); + const url = started.out.match(/QUESTION URL: (\S+)/)?.[1]; + assert.ok(url, `no URL from --start: ${started.out}`); + return { url, payloadPath }; +} + +// Poll --wait until it settles on a terminal exit code (0 answered, 2 gone, +// 4 page closed); loop while it reports WAITING (3). +async function waitLoop(cwd, key, { poll = 30, max = 20 } = {}) { + for (let i = 0; i < max; i++) { + const res = await run(['--wait', '--key', key, '--poll', String(poll)], cwd); + if (res.code !== 3) return res; + } + throw new Error('waitLoop exceeded max iterations'); +} + +async function stopDaemon(cwd, key) { + await run(['--stop', '--key', key], cwd).catch(() => {}); +} + +function makeFakeImage(cwd, prompt, outName) { + const out = path.join(cwd, outName); + const res = spawnSyncGen(prompt, out); + assert.equal(res.status, 0, `generate-image fake failed: ${res.stderr}`); + return out; +} + +function spawnSyncGen(prompt, out, size = null) { + const args = [GENERATE, '--prompt', prompt, '--out', out]; + if (size) args.push('--size', size); + return spawnSync(process.execPath, args, { + env: { ...process.env, IMPECCABLE_IMAGE_GEN_FAKE: '1' }, + encoding: 'buffer', + }); +} + +// -------------------------------------------------------------------------- +// serve-question interactive cycles +// -------------------------------------------------------------------------- +describe('new-work-e2e: serve-question decision page', () => { + it('(a) pick assigned returns the option, hero/board fields, and the CHOSEN CARD directive', async () => { + const cwd = makeWorkspace(); + const key = 'pick'; + const hero = makeFakeImage(cwd, 'Fillmore handbill hero', 'hero.png'); + const board = makeFakeImage(cwd, 'Fillmore handbill board', 'board.png'); + const payload = { + title: 'Choose the visual world', + question: 'The roll assigned Fillmore Handbill.', + options: [ + { id: 'assigned', label: 'Fillmore Handbill', kicker: 'THE ROLL', hero, board }, + { id: 'challenger-teletext', label: 'Teletext Service', body: 'block-mosaic pages' }, + ], + reroll: true, + canon: true, + steer: true, + }; + await startDaemon(cwd, payload, key); + try { + const bot = await runUserBot({ + workspaceDir: cwd, key, browser, + policy: [{ pick: 'assigned', steer: 'warmer palette' }], + }); + assert.equal(bot.results[0].action, 'pick'); + const collected = await waitLoop(cwd, key); + assert.equal(collected.code, 0, collected.out); + assert.match(collected.out, /ANSWER: /); + const answer = JSON.parse(collected.out.match(/ANSWER: (\{.*\})/)[1]); + assert.equal(answer.optionId, 'assigned'); + assert.equal(answer.steer, 'warmer palette'); + assert.ok(answer.hero, 'answer carries the chosen hero path'); + assert.ok(answer.board, 'answer carries the chosen board path'); + assert.match(collected.out, /CHOSEN CARD:/); + } finally { + await stopDaemon(cwd, key); + rmSync(cwd, { recursive: true, force: true }); + } + }); + + it('(b) re-roll with steer keeps the server alive; --update re-deals; the next pick is terminal', async () => { + const cwd = makeWorkspace(); + const key = 'reroll'; + const payload1 = { + title: 'Choose the visual world', + options: [ + { id: 'assigned', label: 'First Hand', kicker: 'THE ROLL' }, + { id: 'challenger-a', label: 'Alt One' }, + ], + reroll: true, steer: true, canon: true, + }; + const payload2 = { + title: 'Choose the visual world', + options: [ + { id: 'assigned', label: 'Second Hand', kicker: 'THE ROLL' }, + { id: 'challenger-b', label: 'Alt Two' }, + ], + reroll: true, steer: true, + }; + await startDaemon(cwd, payload1, key); + try { + // Bot drives the whole page: re-roll (with steer) then, after the page + // reloads into the next hand, pick the assigned card. + const botPromise = runUserBot({ + workspaceDir: cwd, key, browser, + policy: [{ reroll: true, steer: 'colder, more restraint' }, { pick: 'assigned' }], + }); + + // First answer: the re-roll. Server must stay alive afterwards. + const first = await waitLoop(cwd, key); + assert.equal(first.code, 0, first.out); + assert.match(first.out, /"optionId":"reroll"/); + assert.match(first.out, /colder, more restraint/); + assert.ok(existsSync(path.join(cwd, '.impeccable', 'questions', `${key}.state.json`)), + 'server state file survives a re-roll'); + + // Deliver the next hand; the live page reloads itself. + const nextPayloadPath = path.join(cwd, 'next.json'); + writeFileSync(nextPayloadPath, JSON.stringify(payload2)); + const updated = await run(['--update', '--key', key, '--payload', nextPayloadPath], cwd); + assert.equal(updated.code, 0, updated.out); + + // Second answer: the terminal pick on the re-dealt hand. + const second = await waitLoop(cwd, key); + assert.equal(second.code, 0, second.out); + assert.match(second.out, /"optionId":"assigned"/); + + const bot = await botPromise; + assert.equal(bot.results[0].action, 'reroll'); + assert.ok(bot.results[0].reloaded, 'page reloaded into the next hand'); + assert.equal(bot.results[1].action, 'pick'); + + // Terminal pick cleans the state file up. + assert.ok(!existsSync(path.join(cwd, '.impeccable', 'questions', `${key}.state.json`)), + 'terminal pick removes the server state file'); + } finally { + await stopDaemon(cwd, key); + rmSync(cwd, { recursive: true, force: true }); + } + }); + + it('(c) canon click returns optionId canon and prints the CANON CHOSEN directive', async () => { + const cwd = makeWorkspace(); + const key = 'canon'; + const payload = { + title: 'Choose the visual world', + options: [{ id: 'assigned', label: 'Fillmore Handbill', kicker: 'THE ROLL' }], + reroll: true, canon: true, steer: true, + }; + await startDaemon(cwd, payload, key); + try { + await runUserBot({ workspaceDir: cwd, key, browser, policy: [{ canon: true }] }); + const collected = await waitLoop(cwd, key); + assert.equal(collected.code, 0, collected.out); + assert.match(collected.out, /"optionId":"canon"/); + assert.match(collected.out, /CANON CHOSEN:/); + } finally { + await stopDaemon(cwd, key); + rmSync(cwd, { recursive: true, force: true }); + } + }); + + it('(d) closing the tab makes --wait exit 4 PAGE CLOSED', async () => { + const cwd = makeWorkspace(); + const key = 'close'; + const payload = { + title: 'Choose the visual world', + options: [{ id: 'assigned', label: 'Fillmore Handbill', kicker: 'THE ROLL' }], + reroll: true, steer: true, + }; + await startDaemon(cwd, payload, key); + try { + const bot = await runUserBot({ workspaceDir: cwd, key, browser, policy: [{ close: true }] }); + assert.equal(bot.results[0].action, 'close'); + assert.ok(bot.results[0].beat > 0, 'a heartbeat landed before the tab closed'); + // --wait must observe the stale heartbeat and report the closed page. + const res = await run(['--wait', '--key', key, '--poll', '30'], cwd); + assert.equal(res.code, 4, `expected exit 4, got ${res.code}: ${res.out}`); + assert.match(res.out, /PAGE CLOSED/); + } finally { + await stopDaemon(cwd, key); + rmSync(cwd, { recursive: true, force: true }); + } + }); + + it('(e) an option with no hero renders a text-only card (no .media element)', async () => { + const cwd = makeWorkspace(); + const key = 'textonly'; + const hero = makeFakeImage(cwd, 'has a hero', 'hero.png'); + const payload = { + title: 'Choose the visual world', + options: [ + { id: 'assigned', label: 'Text Only Direction', body: 'a grounded direction, no comp' }, + { id: 'challenger-hero', label: 'Has A Card', hero }, + ], + reroll: true, steer: true, + }; + const { url } = await startDaemon(cwd, payload, key); + try { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(url, { waitUntil: 'load' }); + await page.waitForSelector('button.choose'); + const textOnlyMedia = await page.$('.card[data-id="assigned"] .media'); + const heroMedia = await page.$('.card[data-id="challenger-hero"] .media'); + const textOnlyFace = await page.$('.card[data-id="assigned"] .face.text-only'); + await context.close(); + assert.equal(textOnlyMedia, null, 'text-only card has no .media region'); + assert.ok(textOnlyFace, 'text-only card carries the .text-only face class'); + assert.ok(heroMedia, 'the hero card still renders its .media region'); + } finally { + await stopDaemon(cwd, key); + rmSync(cwd, { recursive: true, force: true }); + } + }); +}); + +// -------------------------------------------------------------------------- +// Fake image generation +// -------------------------------------------------------------------------- +describe('new-work-e2e: fake image generation', () => { + it('is deterministic per prompt and encodes the SYNTHETIC marker', () => { + const cwd = mkdtempSync(path.join(tmpdir(), 'new-work-img-')); + try { + const a = path.join(cwd, 'a.png'); + const b = path.join(cwd, 'b.png'); + const r1 = spawnSyncGen('Fillmore psychedelic handbill, warm ink', a); + const r2 = spawnSyncGen('Fillmore psychedelic handbill, warm ink', b); + assert.equal(r1.status, 0, r1.stderr?.toString()); + assert.equal(r2.status, 0, r2.stderr?.toString()); + assert.ok(existsSync(a) && existsSync(b), 'both files exist'); + assert.match(r1.stdout.toString(), /\$0\.00/, 'cost line reads $0.00'); + const bytesA = readFileSync(a); + const bytesB = readFileSync(b); + assert.ok(bytesA.equals(bytesB), 'same prompt yields identical bytes'); + // Valid PNG signature + the SYNTHETIC marker (in the tEXt chunk). + assert.equal(bytesA.slice(0, 8).toString('hex'), '89504e470d0a1a0a'); + assert.ok(bytesA.includes(Buffer.from('SYNTHETIC')), 'PNG carries the SYNTHETIC marker'); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }); + + it('renders a different palette for a different prompt', () => { + const cwd = mkdtempSync(path.join(tmpdir(), 'new-work-img-')); + try { + const a = path.join(cwd, 'a.png'); + const c = path.join(cwd, 'c.png'); + spawnSyncGen('Fillmore psychedelic handbill, warm ink', a); + spawnSyncGen('Teletext broadcast mosaic, cold blue', c); + const bytesA = readFileSync(a); + const bytesC = readFileSync(c); + assert.ok(!bytesA.equals(bytesC), 'different prompts produce different images'); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }); + + it('the SVG variant carries the readable prompt text and SYNTHETIC COMP label', () => { + const cwd = mkdtempSync(path.join(tmpdir(), 'new-work-img-')); + try { + const svg = path.join(cwd, 'comp.svg'); + const res = spawnSyncGen('teletext broadcast mosaic', svg, '800x600'); + assert.equal(res.status, 0, res.stderr?.toString()); + const text = readFileSync(svg, 'utf8'); + assert.match(text, /^<\?xml/, 'is an SVG document'); + assert.match(text, /SYNTHETIC COMP/); + assert.match(text, /teletext/i, 'the prompt text is rendered'); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/new-work-e2e/README.md b/tests/new-work-e2e/README.md new file mode 100644 index 000000000..8f1561447 --- /dev/null +++ b/tests/new-work-e2e/README.md @@ -0,0 +1,65 @@ +# new-work E2E + +A cheap, deterministic smoke suite for the interactive parts of new-work: the +serve-question decision page and the offline image generator. It is kept out of +`bun run test` and runs on demand. + +```bash +bun run test:new-work-e2e +``` + +One-time setup: `npx playwright install chromium` (the suite drives a real +Chromium so the page runs its own JS, exactly as a user's tab would). + +## What it covers + +`tests/new-work-e2e.test.mjs` opens the served decision page with a real +browser and drives it through the scripted user bot, then asserts on the +serve-question protocol output: + +- **pick assigned** returns the chosen `optionId`, the typed steer, the + `hero`/`board` fields, and the `CHOSEN CARD` directive printed by `--wait`. +- **re-roll with steer** keeps the daemon alive, `--update` re-deals the next + hand, the page reloads itself, and the following pick is terminal (state file + cleaned up). +- **canon** returns `optionId: canon` and prints the `CANON CHOSEN` directive. +- **tab close** stops the page heartbeats so `--wait` exits 4 `PAGE CLOSED`. +- **text-only card** renders with no `.media` region when an option has no hero. +- **fake image generation**: same prompt yields identical bytes, the file + exists, the `SYNTHETIC` marker is present, and different prompts produce + different palettes. + +The concept-seed direction roll (challengers, `ASSIGNED INDEX`, the no +PRODUCT.md gate) is already covered by `tests/concept-seed.test.mjs` and is not +repeated here. + +## Pieces + +- `user-bot.mjs` is a module plus CLI. Given a workspace dir it resolves the + running daemon from `.impeccable/questions/.state.json`, opens the page, + and runs a JSON policy of real clicks: `{"pick":"assigned"}`, + `{"reroll":true,"steer":"warmer"}`, `{"pick":"challenger-*"}`, + `{"canon":true}`, `{"close":true}`. The deterministic tier passes an + already-launched browser in; the CLI launches its own Chromium. +- `IMPECCABLE_IMAGE_GEN_FAKE=1` switches `skill/scripts/generate-image.mjs` to + the offline stand-in: no OpenAI call, no key, a `$0.00` cost line, and a + deterministic image (SVG for `.svg` out with the wrapped prompt text and a + `SYNTHETIC COMP` label; a valid palette-stripe PNG otherwise, with the prompt + and marker in a PNG `tEXt` chunk). + +## Planned LLM tier (not built yet) + +The same scaffolding supports an opt-in LLM tier later, mirroring the two-layer +pattern in `tests/live-e2e`: + +- A real model plays the user through the same scripted `user-bot.mjs` policy, + choosing and steering instead of following canned actions. +- `IMPECCABLE_IMAGE_GEN_FAKE` still stands in for image spend, so a full + concept-to-card cycle runs without paying per render. +- Assertions run against the tool-call trace via the skill-behavior harness, + the same way `tests/skill-behavior` keys on the trace rather than free-form + output. + +Cost posture: the deterministic tier is free (no API calls, local Chromium). +The LLM tier hits a provider and costs money, so it stays opt-in and out of CI, +matching how `test:live-e2e` and `test:skill-behavior` are gated today. diff --git a/tests/new-work-e2e/user-bot.mjs b/tests/new-work-e2e/user-bot.mjs new file mode 100644 index 000000000..2fbae07ae --- /dev/null +++ b/tests/new-work-e2e/user-bot.mjs @@ -0,0 +1,196 @@ +/** + * Scripted user bot for the new-work interactive smoke suite. + * + * Given a workspace directory, it discovers a running serve-question daemon + * from `.impeccable/questions/.state.json`, opens the served page in a + * real browser, and drives it through a scripted policy: it clicks the real + * `button.choose`, `#reroll`, and `#canon` controls, types into `#steer`, and + * closes the tab for the exit-4 path. Because a real page runs the page's own + * JS, heartbeats fire and re-roll reloads behave exactly as a user's would. + * + * The deterministic tier passes an already-launched Playwright browser in. + * Run as a CLI (`--workspace DIR --policy ''`) it launches its own + * Chromium. The policy is an ordered list of actions: + * + * { "pick": "assigned" } click the assigned card + * { "pick": "challenger-*" } click the first matching card + * { "pickIndex": 1 } click the Nth choose button + * { "reroll": true, "steer": "warmer" } type the steer, click Re-roll + * { "canon": true } click Play it straight + * { "close": true } close the tab (stops heartbeats) + * + * A `steer` on any action is typed into `#steer` first when the field exists. + * After a re-roll the bot waits for the page to reload into the next hand + * (delivered out of band by `serve-question --update`) before the next action. + */ + +import { readdirSync, readFileSync, existsSync } from 'node:fs'; +import path from 'node:path'; + +function questionsDir(workspaceDir) { + return path.join(workspaceDir, '.impeccable', 'questions'); +} + +// Resolve the served URL from the daemon state file. When no key is given and +// several exist, the newest wins. +export function resolveQuestion(workspaceDir, key = null) { + const dir = questionsDir(workspaceDir); + if (!existsSync(dir)) throw new Error(`no questions dir at ${dir}`); + const stateFiles = readdirSync(dir).filter((f) => f.endsWith('.state.json')); + if (stateFiles.length === 0) throw new Error(`no *.state.json in ${dir}`); + let file; + if (key) { + file = `${key}.state.json`; + if (!stateFiles.includes(file)) throw new Error(`no state file for key ${key}`); + } else { + file = stateFiles + .map((f) => ({ f, mtime: readFileSync(path.join(dir, f), 'utf8') && f })) + .sort() + .pop().f; + } + const resolvedKey = file.replace(/\.state\.json$/, ''); + const state = JSON.parse(readFileSync(path.join(dir, file), 'utf8')); + return { key: resolvedKey, url: state.url, port: state.port, pid: state.pid }; +} + +function stateLastBeat(workspaceDir, key) { + try { + const state = JSON.parse(readFileSync(path.join(questionsDir(workspaceDir), `${key}.state.json`), 'utf8')); + return state.lastBeat || 0; + } catch { + return 0; + } +} + +async function typeSteer(page, action) { + if (action.steer == null) return; + const steer = await page.$('#steer'); + if (steer) await steer.fill(String(action.steer)); +} + +function chooseSelector(pick) { + if (pick.endsWith('*')) { + const prefix = pick.slice(0, -1); + return `button.choose[data-id^="${prefix}"]`; + } + return `button.choose[data-id="${pick}"]`; +} + +async function runAction(page, action, { workspaceDir, key }) { + await typeSteer(page, action); + + if (action.reroll) { + await Promise.all([ + page.waitForNavigation({ waitUntil: 'load', timeout: 60000 }).catch(() => {}), + page.click('#reroll'), + ]); + // Fresh hand loaded: wait for the interactive controls of the next round. + await page.waitForSelector('button.choose', { timeout: 30000 }); + return { action: 'reroll', reloaded: true }; + } + + if (action.canon) { + await page.click('#canon'); + return { action: 'canon' }; + } + + if (action.close) { + // Make sure at least one heartbeat has been recorded so the --wait poll can + // later see the beat go stale (the exit-4 PAGE CLOSED path). + const deadline = Date.now() + 8000; + while (Date.now() < deadline && !stateLastBeat(workspaceDir, key)) { + await page.waitForTimeout(200); + } + await page.close(); + return { action: 'close', beat: stateLastBeat(workspaceDir, key) }; + } + + if (action.pickIndex != null) { + const buttons = await page.$$('button.choose'); + const btn = buttons[action.pickIndex]; + if (!btn) throw new Error(`no choose button at index ${action.pickIndex}`); + await btn.click(); + return { action: 'pick', index: action.pickIndex }; + } + + if (action.pick) { + await page.click(chooseSelector(action.pick)); + return { action: 'pick', id: action.pick }; + } + + throw new Error(`unknown action: ${JSON.stringify(action)}`); +} + +/** + * Drive the served question page through the policy. Pass a launched + * Playwright `browser` (deterministic tier) or omit it to launch Chromium. + */ +export async function runUserBot({ workspaceDir, key = null, policy = [], browser = null }) { + let ownBrowser = null; + let pw = null; + if (!browser) { + pw = await import('playwright'); + ownBrowser = await pw.chromium.launch({ headless: true }); + browser = ownBrowser; + } + const question = resolveQuestion(workspaceDir, key); + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(question.url, { waitUntil: 'load' }); + await page.waitForSelector('button.choose', { timeout: 30000 }); + + const results = []; + let closed = false; + try { + for (const action of policy) { + const result = await runAction(page, action, { workspaceDir, key: question.key }); + results.push(result); + if (result.action === 'close') { closed = true; break; } + // Give the answer POST time to land before the process may exit. + if (result.action === 'pick' || result.action === 'canon') { + await page.waitForTimeout(300); + } + } + } finally { + if (!closed) await context.close().catch(() => {}); + if (ownBrowser) await ownBrowser.close().catch(() => {}); + } + return { key: question.key, url: question.url, results }; +} + +// -------------------------------------------------------------------------- +// CLI +// -------------------------------------------------------------------------- +function cliArg(name, fallback = null) { + const i = process.argv.indexOf(`--${name}`); + if (i === -1) return fallback; + const v = process.argv[i + 1]; + return v && !v.startsWith('--') ? v : fallback; +} + +const isMain = import.meta.url === `file://${process.argv[1]}`; +if (isMain) { + const workspaceDir = cliArg('workspace'); + const key = cliArg('key'); + const policyRaw = cliArg('policy'); + if (!workspaceDir || !policyRaw) { + console.error('user-bot: --workspace and --policy are required.'); + process.exit(1); + } + let policy; + try { + policy = JSON.parse(policyRaw); + } catch (err) { + console.error(`user-bot: --policy must be JSON (${err.message})`); + process.exit(1); + } + runUserBot({ workspaceDir, key, policy }) + .then((out) => { + console.log(JSON.stringify(out)); + process.exit(0); + }) + .catch((err) => { + console.error(`user-bot: ${err.message}`); + process.exit(1); + }); +} diff --git a/tests/skills-cli.test.js b/tests/skills-cli.test.js index bb16beeb4..05c33eb36 100644 --- a/tests/skills-cli.test.js +++ b/tests/skills-cli.test.js @@ -98,8 +98,11 @@ function createFakeUniversalBundle(root, providers = ['.claude', '.agents', '.cu } if (providers.includes('.agents')) { mkdirSync(join(bundleRoot, '.codex'), { recursive: true }); + // Mirror production: the Codex bundle's `.codex/hooks.json` targets its own + // `.codex/skills` payload. The CLI installs the skill at `.agents/skills`, so + // the installer must rewrite this command to `.agents/skills` (see below). writeFileSync(join(bundleRoot, '.codex', 'hooks.json'), JSON.stringify({ - hooks: { PostToolUse: [{ matcher: 'apply_patch', hooks: [{ type: 'command', command: 'node ".agents/skills/impeccable/scripts/hook.mjs"' }] }] }, + hooks: { PostToolUse: [{ matcher: 'apply_patch', hooks: [{ type: 'command', command: 'node ".codex/skills/impeccable/scripts/hook.mjs"' }] }] }, }, null, 2)); } return bundleRoot; @@ -675,6 +678,12 @@ describe('skills install/update: local universal bundle e2e', () => { expect(existsSync(join(tmp, '.claude', 'settings.local.json'))).toBe(true); expect(existsSync(join(tmp, '.cursor', 'hooks.json'))).toBe(true); expect(existsSync(join(tmp, '.codex', 'hooks.json'))).toBe(true); + // The CLI puts Codex's skill at `.agents/skills`, so the project-scope hook + // command must point there — not at the bundle's own `.codex/skills` path, + // which would resolve to a nonexistent file and silently no-op the hook. + const codexHooks = readFileSync(join(tmp, '.codex', 'hooks.json'), 'utf8'); + expect(codexHooks).toContain('.agents/skills/impeccable/scripts/hook.mjs'); + expect(codexHooks).not.toContain('.codex/skills/impeccable/scripts/hook.mjs'); rmSync(tmp, { recursive: true, force: true }); }, 15000); @@ -875,6 +884,145 @@ describe('skills install/update: local universal bundle e2e', () => { rmSync(home, { recursive: true, force: true }); }, 15000); + // OpenCode reads global skills from its config directory, not ~/.opencode: + // $OPENCODE_CONFIG_DIR/skills, else $XDG_CONFIG_HOME/opencode/skills, else + // ~/.config/opencode/skills. Writing to ~/.opencode/skills produced an + // install `opencode debug skill` never saw (#406). + test('global install writes OpenCode skills to ~/.config/opencode/skills (#406)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-scope-user-oc-')); + const home = mkdtempSync(join(tmpdir(), 'imp-home-scope-user-oc-')); + execSync('git init', { cwd: tmp }); + mkdirSync(join(home, '.opencode'), { recursive: true }); + const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + delete env.OPENCODE_CONFIG_DIR; + delete env.XDG_CONFIG_HOME; + + const output = run('skills install -y --scope=global --no-hooks', { cwd: tmp, env }); + + expect(output).toContain('Installed impeccable into: .opencode (global)'); + expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + expect(existsSync(join(home, '.opencode', 'skills', 'impeccable'))).toBe(false); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 15000); + + test('OpenCode global dir honors OPENCODE_CONFIG_DIR and XDG_CONFIG_HOME (#406)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-env-')); + const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-env-')); + execSync('git init', { cwd: tmp }); + const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']); + const baseEnv = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + delete baseEnv.OPENCODE_CONFIG_DIR; + delete baseEnv.XDG_CONFIG_HOME; + + run('skills install -y --providers=opencode --scope=global --no-hooks', { + cwd: tmp, + env: { ...baseEnv, OPENCODE_CONFIG_DIR: join(home, 'occfg') }, + }); + expect(existsSync(join(home, 'occfg', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + + run('skills install -y --providers=opencode --scope=global --no-hooks', { + cwd: tmp, + env: { ...baseEnv, XDG_CONFIG_HOME: join(home, 'xdg') }, + }); + expect(existsSync(join(home, 'xdg', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + expect(existsSync(join(home, '.opencode', 'skills', 'impeccable'))).toBe(false); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 20000); + + test('global OpenCode install migrates a legacy ~/.opencode/skills copy, sparing siblings (#406)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-migrate-')); + const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-migrate-')); + execSync('git init', { cwd: tmp }); + writeSkill(home, '.opencode', 'impeccable'); + writeSkill(home, '.opencode', 'unrelated-skill'); + const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + delete env.OPENCODE_CONFIG_DIR; + delete env.XDG_CONFIG_HOME; + + run('skills install -y --providers=opencode --scope=global --no-hooks', { cwd: tmp, env }); + + expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + // The stranded legacy copy is gone; the sibling skill is untouched. + expect(existsSync(join(home, '.opencode', 'skills', 'impeccable'))).toBe(false); + expect(existsSync(join(home, '.opencode', 'skills', 'unrelated-skill', 'SKILL.md'))).toBe(true); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 15000); + + test('OpenCode migration never follows a symlinked legacy skills dir (#406)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-symlink-')); + const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-symlink-')); + execSync('git init', { cwd: tmp }); + // Shared skill storage with ~/.opencode/skills symlinked at it. Deleting + // "the legacy copy" through the link would destroy the shared original. + writeSkill(join(home, '.config'), 'agents', 'impeccable'); + mkdirSync(join(home, '.opencode'), { recursive: true }); + symlinkSync(join(home, '.config', 'agents', 'skills'), join(home, '.opencode', 'skills'), 'dir'); + const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + delete env.OPENCODE_CONFIG_DIR; + delete env.XDG_CONFIG_HOME; + + run('skills install -y --providers=opencode --scope=global --no-hooks', { cwd: tmp, env }); + + expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + // The shared store behind the symlink is intact, link included. + expect(existsSync(join(home, '.config', 'agents', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + expect(lstatSync(join(home, '.opencode', 'skills')).isSymbolicLink()).toBe(true); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 15000); + + test('OpenCode migration leaves a home-rooted repo project install alone (#406)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-homerepo-')); + const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-homerepo-')); + execSync('git init', { cwd: tmp }); + // The home dir IS a repo (dotfiles setup): .opencode/skills there is a + // live project-scope install, not a stranded pre-#406 global one. + execSync('git init', { cwd: home }); + writeSkill(home, '.opencode', 'impeccable'); + const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + delete env.OPENCODE_CONFIG_DIR; + delete env.XDG_CONFIG_HOME; + + run('skills install -y --providers=opencode --scope=global --no-hooks', { cwd: tmp, env }); + + expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + expect(existsSync(join(home, '.opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 15000); + + test('global install detects OpenCode from ~/.config/opencode alone (#406)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-detect-')); + const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-detect-')); + execSync('git init', { cwd: tmp }); + // No ~/.opencode at all; only the config dir marks OpenCode as present. + mkdirSync(join(home, '.config', 'opencode'), { recursive: true }); + const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + delete env.OPENCODE_CONFIG_DIR; + delete env.XDG_CONFIG_HOME; + + const output = run('skills install -y --scope=global --no-hooks', { cwd: tmp, env }); + + expect(output).toContain('Installed impeccable into: .opencode (global)'); + expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 15000); + // Project scope must stay at .pi/skills/ even when the git root IS the home // dir (dotfiles repos), where scope can't be inferred from the path alone. // An existing global install at ~/.pi/agent/skills must not swallow the @@ -1253,6 +1401,154 @@ describe('hook manifest merge helpers', () => { }); }); +// ─── Hook command path resolution (issue #399, part 1) ─────────────────────── +// The bundled Claude manifest ships a ${CLAUDE_PROJECT_DIR}-relative command. +// That resolves per-project, so a user-level (~/.claude/settings.local.json) +// hook — which fires in EVERY project — must be rewritten to the resolved +// absolute skill path, or Node crashes on every PostToolUse/Stop in projects +// without a local skill copy. Project-level hooks keep ${CLAUDE_PROJECT_DIR}. +// Both are wrapped with a missing-file guard so a missing script exits 0. + +// A bundle whose Claude manifest mirrors production: ${CLAUDE_PROJECT_DIR}-relative. +function createProjectDirBundle(root) { + const bundleRoot = join(root, 'projdir-bundle'); + const skillDir = join(bundleRoot, '.claude', 'skills', 'impeccable', 'scripts'); + mkdirSync(skillDir, { recursive: true }); + writeFileSync(join(bundleRoot, '.claude', 'skills', 'impeccable', 'SKILL.md'), + '---\nname: impeccable\nversion: 9.9.9-local\n---\nbundle\n'); + mkdirSync(join(bundleRoot, '.claude'), { recursive: true }); + writeFileSync(join(bundleRoot, '.claude', 'settings.json'), JSON.stringify({ + hooks: { + PostToolUse: [{ matcher: 'Edit|Write|MultiEdit', hooks: [ + { type: 'command', command: 'node "${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs"' }, + ] }], + Stop: [{ hooks: [ + { type: 'command', command: 'node "${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs"' }, + ] }], + }, + }, null, 2)); + return bundleRoot; +} + +function claudeHookCommands(manifestPath) { + const parsed = JSON.parse(readFileSync(manifestPath, 'utf8')); + return Object.values(parsed.hooks).flatMap(entries => + entries.flatMap(entry => (entry.hooks || []).map(h => h.command))); +} + +describe('copyProviderHooks: hook command path resolution (#399)', () => { + test('project-scope hook keeps ${CLAUDE_PROJECT_DIR} and adds a missing-file guard', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-hook-project-')); + const bundleDir = createProjectDirBundle(tmp); + + // skillRoot === root === a non-home project dir: keep the portable token. + copyProviderHooks(bundleDir, tmp, ['.claude'], { skillRoot: tmp }); + + const commands = claudeHookCommands(join(tmp, '.claude', 'settings.local.json')); + expect(commands.length).toBeGreaterThan(0); + for (const command of commands) { + expect(command).toContain('${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs'); + expect(command).not.toContain(tmp); // no absolute rewrite for project scope + expect(command).toContain('[ ! -f '); // guarded so a missing file exits 0 + expect(command).not.toContain('|| true'); // must preserve node's exit code + } + rmSync(tmp, { recursive: true, force: true }); + }); + + // The user/global case (isHomeDir(root) true) is driven end-to-end through a + // child process below ('user-level update writes an absolute, guarded hook'), + // where HOME is set in the child's env so os.homedir() reflects it. It cannot + // be faked reliably in-process, so it is not unit-tested here. + + test('project hook pointing at a global skill uses the absolute skill path', () => { + // --scope=global shape: manifest root is the project, skill lives in home. + const tmp = mkdtempSync(join(tmpdir(), 'imp-hook-split-')); + const skillHome = mkdtempSync(join(tmpdir(), 'imp-hook-skillroot-')); + const bundleDir = createProjectDirBundle(tmp); + + copyProviderHooks(bundleDir, tmp, ['.claude'], { skillRoot: skillHome }); + + const commands = claudeHookCommands(join(tmp, '.claude', 'settings.local.json')); + const absolute = join(skillHome, '.claude', 'skills', 'impeccable', 'scripts', 'hook.mjs'); + for (const command of commands) { + expect(command).toContain(absolute); + expect(command).not.toContain('${CLAUDE_PROJECT_DIR}'); + expect(command).toContain('[ ! -f '); + } + rmSync(tmp, { recursive: true, force: true }); + rmSync(skillHome, { recursive: true, force: true }); + }); +}); + +// ─── Update scope resolution (issue #399, part 2) ──────────────────────────── + +describe('skills update: names the resolved target and honors scope (#399)', () => { + test('user-level update writes an absolute, guarded hook to ~/.claude', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-update-user-')); + const home = mkdtempSync(join(tmpdir(), 'imp-update-user-home-')); + execSync('git init', { cwd: tmp }); + const bundleRoot = createFakeUniversalBundle(tmp, ['.claude']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + + // Seed a user-level install (skills only), then update it with hooks. + run('skills install -y --providers=claude --scope=global --no-hooks', { cwd: tmp, env }); + expect(existsSync(join(home, '.claude', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + + const output = run('skills update -y --user', { cwd: tmp, env }); + expect(output).toContain('user level'); // scope named explicitly + expect(output).toContain('~'); // resolved home path named, not a bare ".claude" + + const settingsPath = join(home, '.claude', 'settings.local.json'); + expect(existsSync(settingsPath)).toBe(true); + const raw = readFileSync(settingsPath, 'utf8'); + expect(raw).toContain(join(home, '.claude', 'skills', 'impeccable', 'scripts', 'hook.mjs')); + expect(raw).not.toContain('${CLAUDE_PROJECT_DIR}'); + expect(raw).toContain('[ ! -f '); + // The project dir was never touched. + expect(existsSync(join(tmp, '.claude', 'skills', 'impeccable'))).toBe(false); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 20000); + + test('does not vendor impeccable into a repo that only tracks OTHER skills', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-update-vendor-')); + const home = mkdtempSync(join(tmpdir(), 'imp-update-vendor-home-')); + execSync('git init', { cwd: tmp }); + const bundleRoot = createFakeUniversalBundle(tmp, ['.claude']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + + // The repo tracks a first-party, NON-impeccable skill under .claude/skills. + writeSkill(tmp, '.claude', 'house-brand'); + // A real user-level impeccable install exists. + run('skills install -y --providers=claude --scope=global --no-hooks', { cwd: tmp, env }); + + const output = run('skills update -y', { cwd: tmp, env }); + // Targets the user level, not the project's unrelated .claude/skills. + expect(output).toContain('user level'); + expect(existsSync(join(tmp, '.claude', 'skills', 'impeccable'))).toBe(false); + expect(existsSync(join(tmp, '.claude', 'skills', 'house-brand'))).toBe(true); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 20000); + + test('--user with no user-level install reports the resolved user path', () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-update-nouser-')); + const home = mkdtempSync(join(tmpdir(), 'imp-update-nouser-home-')); + execSync('git init', { cwd: tmp }); + const bundleRoot = createFakeUniversalBundle(tmp, ['.claude']); + const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot }; + // Only a project install exists; --user must not fall through to it. + run('skills install -y --providers=claude --no-hooks', { cwd: tmp, env }); + + expect(() => run('skills update -y --user', { cwd: tmp, env, stdio: 'pipe' })).toThrow(); + + rmSync(tmp, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }, 20000); +}); + // ─── Update fallback (remote direct download smoke) ────────────────────────── describeRemote('skills update: refreshes from the production universal bundle', () => {