feat(agent): add windsurf as an install target#29
Conversation
4e02451 to
eea1d02
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughThis PR adds ChangesWindsurf target support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DOCUMENTATION.md (1)
122-125: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winOwn-file target list at Line 124 is missing
windsurf.This PR adds
windsurfas an own-file target (TARGETS.windsurf.mode === 'own-file'), but the--forcebackup-behavior callout still only listsclaude, cursor, cline, antigravity. Windsurf is subject to the same backup-on---forcebehavior and should be included.📝 Proposed fix
-Re-running with `--force` on **own-file targets** (claude, cursor, cline, antigravity) backs up the existing file to `<path>.bak` first. +Re-running with `--force` on **own-file targets** (claude, cursor, cline, windsurf, antigravity) backs up the existing file to `<path>.bak` first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DOCUMENTATION.md` around lines 122 - 125, The own-file target backup note is incomplete because `windsurf` is now an own-file target too. Update the `DOCUMENTATION.md` guidance around the `--force` behavior so the own-file target list explicitly includes `windsurf` alongside `claude`, `cursor`, `cline`, and `antigravity`, keeping the backup-on-`--force` description consistent with `TARGETS.windsurf` and the other own-file targets.
🧹 Nitpick comments (1)
test/e2e/agent-install.e2e.test.ts (1)
164-168: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winComment claims "description" is verified, but assertion only checks
trigger.The comment says
// Windsurf Cascade frontmatter: trigger + description (no name/alwaysApply), but no assertion verifies adescription:field is present, even though the PR requirement explicitly calls fordescription: <skill description>in the frontmatter.✏️ Suggested addition
} else if (target === 'windsurf') { // Windsurf Cascade frontmatter: trigger + description (no name/alwaysApply) expect(content.startsWith('---'), `windsurf: should start with ---`).toBe(true); expect(content).toContain('trigger: model_decision'); + expect(content).toContain('description:'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/agent-install.e2e.test.ts` around lines 164 - 168, The Windsurf frontmatter check in the e2e test only verifies trigger and does not assert the required description field, so update the `agent-install.e2e.test.ts` expectations in the `target === 'windsurf'` branch to confirm the generated content includes a `description:` entry with the skill description. Keep the existing `trigger: model_decision` assertion and add a matching check for the `description` field so the test aligns with the `Windsurf Cascade frontmatter` requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@DOCUMENTATION.md`:
- Around line 122-125: The own-file target backup note is incomplete because
`windsurf` is now an own-file target too. Update the `DOCUMENTATION.md` guidance
around the `--force` behavior so the own-file target list explicitly includes
`windsurf` alongside `claude`, `cursor`, `cline`, and `antigravity`, keeping the
backup-on-`--force` description consistent with `TARGETS.windsurf` and the other
own-file targets.
---
Nitpick comments:
In `@test/e2e/agent-install.e2e.test.ts`:
- Around line 164-168: The Windsurf frontmatter check in the e2e test only
verifies trigger and does not assert the required description field, so update
the `agent-install.e2e.test.ts` expectations in the `target === 'windsurf'`
branch to confirm the generated content includes a `description:` entry with the
skill description. Keep the existing `trigger: model_decision` assertion and add
a matching check for the `description` field so the test aligns with the
`Windsurf Cascade frontmatter` requirement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5aca309-64c3-4edb-b169-8b9c5234b940
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
DOCUMENTATION.mdREADME.mdsrc/commands/agent.test.tssrc/commands/agent.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.tstest/e2e/agent-install.e2e.test.tstest/e2e/setup.e2e.test.ts
eea1d02 to
3f851ab
Compare
|
Reworked this onto the v0.2.0 multi-skill agent-targets API (
Note on the v0.2.0 budget handling: own-file targets now install multiple skills as separate files, and Windsurf rules files cap at ~12 K characters. So Windsurf renders the compact body per skill — the trimmed codex asset for |
|
Target policy settled: windsurf is accepted (see #28). This will get review in the next wave. |
|
Approved — no changes requested on substance. This went CONFLICTING purely because today's merge wave landed a large batch into the same files first. Rebase onto current |
Windsurf (Cascade) reads workspace rules from `.windsurf/rules/*.md`. Add it as an own-file agent target so `testsprite agent install --target windsurf` (and `setup --agent windsurf`) installs the TestSprite skills into a Windsurf project. Reworked onto the v0.2.0 multi-skill agent-targets API (pathFor / SKILLS / DEFAULT_SKILLS). Rule files use Cascade frontmatter with `trigger: model_decision` — the equivalent of the Cursor `.mdc` `alwaysApply: false` mode (description shown up front; full body pulled in on relevance). Budget handling: a `.windsurf/rules/*.md` file caps at ~12 K characters and Cascade silently truncates beyond it, which would cut the full ~22 KB verify skill in half. The windsurf target therefore renders the COMPACT body per skill (new `compactBody` flag + `compactBodyFor`): a skill that ships a trimmed codex asset (`testsprite-verify` → ~5 KB) uses it, while a skill whose codex contribution is only a one-liner (`testsprite-onboard`, ~6.5 KB full) keeps its full body — both land well under the cap. `agent.ts` and `renderForTarget` select the same body so installed bytes match the render. Everything else derives from the TARGETS map automatically (agent list, the setup --agent choices, skill-nudge install detection). Updated the hardcoded help strings, the --help snapshot, the agent-targets/agent unit tests (incl. Cascade-frontmatter and per-skill budget tests), the e2e matrix guards / content-integrity (gated on compactBody), and the README/DOCUMENTATION target lists (incl. the --force own-file list).
3f851ab to
a56f783
Compare
|
Rebased onto latest |
|
@zeshi-du kindly review |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/commands/agent.test.ts (1)
952-978: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded target list can silently drift from
OWN_FILE_TARGETS.Line 934's sibling test already switched to
[...OWN_FILE_TARGETS], but this dry-run test still hardcodes['claude', 'cursor', 'cline', 'antigravity', 'kiro', 'windsurf']and the "12 would-write lines" / "six own-file targets" comments. If a new own-file target is added later, this test won't pick it up automatically and will keep passing with a stale count instead of catching the regression.♻️ Suggested fix for consistency with the runInstall test above
- target: ['claude', 'cursor', 'cline', 'antigravity', 'kiro', 'windsurf'], + target: [...OWN_FILE_TARGETS],- // 6 targets × 2 default skills = 12 would-write lines - const wouldWriteLines = stderrOut.split('\n').filter(l => l.includes('would write')); - expect(wouldWriteLines.length).toBe(12); + // OWN_FILE_TARGETS.length targets × 2 default skills = would-write lines + const wouldWriteLines = stderrOut.split('\n').filter(l => l.includes('would write')); + expect(wouldWriteLines.length).toBe(OWN_FILE_TARGETS.length * DEFAULT_SKILLS.length);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/agent.test.ts` around lines 952 - 978, The dry-run test is hardcoding the own-file target list and expected count, which can drift from OWN_FILE_TARGETS over time. Update the runInstall dry-run test to derive the target array from OWN_FILE_TARGETS (as the sibling test does) and compute the expected “would write” line count from that source so it stays aligned when targets change. Use the existing runInstall and OWN_FILE_TARGETS symbols to keep the test consistent and future-proof.src/lib/agent-targets.ts (1)
140-150: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUnescaped description in YAML frontmatter.
wrapWindsurfinterpolatesdescriptiondirectly into an unquoted YAML scalar (description: ${description}). If a skill description ever contains a colon-space, leading special char, or newline, this would produce invalid YAML that Cascade's parser may reject or mis-parse. This mirrors existing wrap helpers for other targets, so it's a pre-existing pattern rather than new, but worth confirming skill descriptions are guaranteed plain and colon-free (or add quoting) so.windsurf/rules/*.mdfrontmatter stays valid as skill content evolves.Optional: quote the YAML value defensively
- return `---\ntrigger: model_decision\ndescription: ${description}\n---\n\n${body}\n`; + return `---\ntrigger: model_decision\ndescription: ${JSON.stringify(description)}\n---\n\n${body}\n`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/agent-targets.ts` around lines 140 - 150, The Windsurf frontmatter builder in wrapWindsurf interpolates description into an unquoted YAML scalar, so make it safe for any description content. Update wrapWindsurf to defensively quote or escape the description before emitting the .windsurf/rules/*.md frontmatter, keeping the YAML valid even if the text contains colons, special characters, or newlines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/commands/agent.test.ts`:
- Around line 952-978: The dry-run test is hardcoding the own-file target list
and expected count, which can drift from OWN_FILE_TARGETS over time. Update the
runInstall dry-run test to derive the target array from OWN_FILE_TARGETS (as the
sibling test does) and compute the expected “would write” line count from that
source so it stays aligned when targets change. Use the existing runInstall and
OWN_FILE_TARGETS symbols to keep the test consistent and future-proof.
In `@src/lib/agent-targets.ts`:
- Around line 140-150: The Windsurf frontmatter builder in wrapWindsurf
interpolates description into an unquoted YAML scalar, so make it safe for any
description content. Update wrapWindsurf to defensively quote or escape the
description before emitting the .windsurf/rules/*.md frontmatter, keeping the
YAML valid even if the text contains colons, special characters, or newlines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f6ebfce0-17f1-431d-8e60-11e363cdc4af
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (8)
DOCUMENTATION.mdREADME.mdsrc/commands/agent.test.tssrc/commands/agent.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.tstest/e2e/agent-install.e2e.test.tstest/e2e/setup.e2e.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- test/e2e/agent-install.e2e.test.ts
- test/e2e/setup.e2e.test.ts
- src/commands/agent.ts
Fixes #138
Describe the changes you have made in this PR -
Adds Windsurf (Codeium's Cascade editor) as an
agent installtarget, sotestsprite agent install --target windsurfandtestsprite setup --agent windsurfwire the verification-loop skill into a Windsurf project.windsurf→.windsurf/rules/testsprite-verify.md.trigger: model_decision+description:— the equivalent of the Cursor.mdcalwaysApply: falsemode (only the description is surfaced up front; Cascade pulls in the full rule body when the description shows it is relevant), which matches how this skill should activate..windsurf/rules/*.mdfile is capped at ~12 K characters and Cascade silently truncates beyond it. The full skill body (~21 KB) would be cut in half, so the windsurf target renders the compact body — the same trimmed variant the Codex/AGENTS.md target already ships (~5 KB) — selected via a newcompactBodyflag on the target spec. The rendered file is ~5.5 KB: well within budget, and it still carries the H1, the when-to-run guidance, and the load-bearingtestsprite test run … --wait/test artifact getcommands.agent.tsandrenderForTargetchoose the same body so the installed bytes equal the asserted render.TARGETSmap automatically (theagent listtable, thesetup --agentchoices, skill-nudge install detection). Updated the two hardcoded help strings inagent.ts, the--helpsnapshot, the unit tests (render test for the Cascade frontmatter + a budget test asserting the windsurf render stays under 12 K and uses the compact body), the e2e matrix guards / multi-target / content-integrity coverage, and the README / DOCUMENTATION target lists.Demo/Screenshot for feature changes and bug fixes -
Tests:
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Problem: TestSprite's value is wiring the verify skill into coding agents, but Windsurf — a top-tier AI editor — wasn't a supported target.
The target system is a clean table (
TARGETSinagent-targets.ts) with a per-targetwrap()for the file's frontmatter, so the core addition is one entry plus a wrap function. I chosetrigger: model_decisionafter checking Windsurf's rules docs: its semantics match the existing Cursor target (alwaysApply: false) — surface the description, load the body on demand — rather thanalways_on(injects the whole skill into every prompt) ormanual(needs an@-mention, defeating an auto-verify skill).The non-obvious part is the size budget. Windsurf caps a
.windsurf/rules/*.mdfile at ~12 K characters and truncates silently beyond it; the full skill body renders to ~22 KB, so half the skill would be lost. The repo already maintains a trimmed/compact body for the Codex/AGENTS.md target, which is agent-agnostic (no Codex- or AGENTS.md-specific wording), so I reused it for Windsurf via acompactBodyflag rather than authoring a third asset. Both therenderForTargetdefault and theagent.tsinstall path branch on this flag (loading each source body at most once), so what gets written equals what the unit test renders.Alternatives considered: (1) reuse
wrapMdc(Cursor's wrapper) — rejected, Cursor'sdescription/alwaysApplykeys are not Windsurf'strigger/description; (2) ship the full body and just emit a budget warning like the Codex target — rejected, because unlike Codex (whose body fits its 32 K budget) the full body always exceeds Windsurf's cap, so it would warn on every install and still truncate; (3) the legacy single-file.windsurfrules— rejected in favour of the current.windsurf/rules/*.mddirectory format.Edge cases handled/tested: a render test asserts the Windsurf output carries
trigger: model_decision+description:and not thename:/alwaysApply:keys; a budget test asserts the real windsurf render is < 12 K characters, uses the compact body (a full-body-only sentence is absent) yet keeps the load-bearing commands, and is smaller than the full claude render; the "install all own-file targets" unit test derives its list fromOWN_FILE_TARGETS; and the e2e content-integrity loop gates the full-body assertion on non-compact targets while adding a Windsurf frontmatter branch.Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
--forcebackup/restore behavior descriptions for own-file targets.Tests