feat(adapters): follow the Windsurf rename to Devin Desktop - #1167
Conversation
- Create new Devin Desktop adapter for .devin/workflows/opsx-<id>.md - Register adapter in CommandAdapterRegistry - Export adapter from adapters index - Update docs/supported-tools.md with Devin Desktop entry - Add 'devin' to available tool IDs list Devin Desktop uses the same Cascade workflow system as Windsurf, making it a natural migration path for existing users.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Devin Desktop support by introducing a new Devin adapter, registering and exporting it, adding ChangesDevin Desktop Tool Support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
openspec/changes/add-devin-desktop-support/specs/cli-update/spec.md (1)
3-19: ⚡ Quick winRename requirement to “Workflow Updates” for consistency with the actual behavior.
This requirement describes refreshing workflow files, not slash commands. Tightening the label avoids ambiguous interpretation in future diffs/tests.
Based on learnings, slash commands and workflow artifacts are distinct concepts in OpenSpec.
🤖 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 `@openspec/changes/add-devin-desktop-support/specs/cli-update/spec.md` around lines 3 - 19, Rename the requirement heading "Requirement: Slash Command Updates" to "Requirement: Workflow Updates" to match the described behavior; update any occurrences of that heading text in this spec file (the top-level requirement title and any references) so tests and readers clearly understand this applies to workflow files (see the heading string "Requirement: Slash Command Updates" and the scenarios mentioning workflows for Devin Desktop and Windsurf).openspec/changes/add-devin-desktop-support/specs/cli-init/spec.md (1)
20-29: ⚡ Quick winUse “workflow” terminology instead of “slash command” for Devin/Windsurf scenarios.
The section header conflicts with the scenarios beneath it (which are workflow files). Renaming to “Workflow Configuration” will keep spec language precise.
Based on learnings, in this repo slash commands are agent-interface commands and should be distinguished from workflow-file generation.
🤖 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 `@openspec/changes/add-devin-desktop-support/specs/cli-init/spec.md` around lines 20 - 29, The header "Requirement: Slash Command Configuration" is inconsistent with the scenarios that generate Devin workflow files; rename that header to "Requirement: Workflow Configuration" (or simply "Workflow Configuration") and update any references in the same spec section so scenarios like "Generating workflows for Devin Desktop" and file names `.devin/workflows/opsx-propose.md`, `.devin/workflows/opsx-apply.md`, `.devin/workflows/opsx-archive.md` are described as workflow-file generation (not agent slash commands), and ensure wording mentions using shared templates wrapped in OpenSpec markers and the Windsurf-style frontmatter (name, description, category, tags).
🤖 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.
Inline comments:
In `@docs/supported-tools.md`:
- Line 33: Docs list 'devin' as a valid tool ID but
parseWorkspaceSkillToolsValue()/getWorkspaceSkillToolIds() derive valid IDs from
AI_TOOLS, which lacks a value 'devin', causing validation to fail; fix by adding
a matching value 'devin' to the corresponding AI_TOOLS entry (the object that
represents Devin Desktop) so getToolsWithSkillsDir()/getWorkspaceSkillToolIds()
will include it, or alternatively update
getToolsWithSkillsDir()/parseWorkspaceSkillToolsValue() to accept the documented
alias; reference AI_TOOLS, getToolsWithSkillsDir(), getWorkspaceSkillToolIds(),
and parseWorkspaceSkillToolsValue() when making the change.
In `@openspec/changes/add-devin-desktop-support/tasks.md`:
- Around line 45-47: The `.devin/skills/openspec-*/SKILL.md` entry appears
inconsistent with the introduced Devin workflow location; open the
"Non-Interactive Setup" section and either correct or remove that skills path so
it matches the actual Devin artifact layout (e.g., if skills live elsewhere,
replace `.devin/skills/openspec-*/SKILL.md` with the correct path, or drop it if
no skills directory is used), and ensure the tools list still includes `devin`
only where appropriate (referencing the `opsx-<id>.md` workflow entry and the
`devin` tool ID).
In `@src/core/command-generation/adapters/devin.ts`:
- Around line 15-23: escapeYamlValue currently leaves implicit-typed YAML
scalars (e.g., true/false, null, yes/no, on/off, plain numbers, hex/exponential)
unquoted which lets parsers coerce them; update escapeYamlValue to detect these
scalar patterns in addition to special chars and force quoting: add a regex
check for booleans (true|false|yes|no|on|off), null variants (null|~), numeric
forms (integers, floats, exponentials, hex/octal), and standalone dot/minus
forms, and if matched, perform the existing escaping (backslashes, quotes,
newlines) and return the value wrapped in double quotes so the result always
remains a string in YAML frontmatter while preserving current escaping logic.
---
Nitpick comments:
In `@openspec/changes/add-devin-desktop-support/specs/cli-init/spec.md`:
- Around line 20-29: The header "Requirement: Slash Command Configuration" is
inconsistent with the scenarios that generate Devin workflow files; rename that
header to "Requirement: Workflow Configuration" (or simply "Workflow
Configuration") and update any references in the same spec section so scenarios
like "Generating workflows for Devin Desktop" and file names
`.devin/workflows/opsx-propose.md`, `.devin/workflows/opsx-apply.md`,
`.devin/workflows/opsx-archive.md` are described as workflow-file generation
(not agent slash commands), and ensure wording mentions using shared templates
wrapped in OpenSpec markers and the Windsurf-style frontmatter (name,
description, category, tags).
In `@openspec/changes/add-devin-desktop-support/specs/cli-update/spec.md`:
- Around line 3-19: Rename the requirement heading "Requirement: Slash Command
Updates" to "Requirement: Workflow Updates" to match the described behavior;
update any occurrences of that heading text in this spec file (the top-level
requirement title and any references) so tests and readers clearly understand
this applies to workflow files (see the heading string "Requirement: Slash
Command Updates" and the scenarios mentioning workflows for Devin Desktop and
Windsurf).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1310b76-ad6a-4ea0-bb86-f47e4f61ad1a
📒 Files selected for processing (9)
docs/supported-tools.mdopenspec/changes/add-devin-desktop-support/.openspec.yamlopenspec/changes/add-devin-desktop-support/proposal.mdopenspec/changes/add-devin-desktop-support/specs/cli-init/spec.mdopenspec/changes/add-devin-desktop-support/specs/cli-update/spec.mdopenspec/changes/add-devin-desktop-support/tasks.mdsrc/core/command-generation/adapters/devin.tssrc/core/command-generation/adapters/index.tssrc/core/command-generation/registry.ts
Add Devin Desktop entry to AI_TOOLS configuration so that: - getToolsWithSkillsDir() includes 'devin' as a valid tool ID - getWorkspaceSkillToolIds() returns 'devin' in the list - parseWorkspaceSkillToolsValue() accepts 'devin' as valid input - openspec init --tools devin works correctly This fixes validation failures where 'devin' was documented in docs/supported-tools.md but not recognized by validation functions that derive valid IDs from AI_TOOLS.
Update escapeYamlValue to detect and quote implicit YAML scalars that would be coerced by parsers: - Booleans: true, false, yes, no, on, off - Null variants: null, ~ - Numbers: integers, floats, exponentials, hex (0x), octal (0o) - Edge cases: standalone dash (-) and dot (.) This ensures values like 'true', '123', 'null' remain strings in YAML frontmatter instead of being interpreted as booleans, numbers, or nulls. Preserves existing escaping logic for special characters and newlines.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for adding this. The registry/config wiring is mostly in the right shape, and I verified openspec init --tools devin generates .devin/skills/* plus .devin/workflows/opsx-*.md successfully.
I’m going to request changes before merge for two reasons:
-
Devin workflows are flat slash workflows, but the generated bodies still point users at colon commands like
/opsx:applyand describe input after/opsx:propose. Devin Desktop discovers.devin/workflows/opsx-apply.mdas/opsx-apply, same as the existing Windsurf shape. Please either run the body through the hyphen command-reference transformer for the Devin adapter, or make the adapter explicitly follow whatever invocation syntax Devin actually accepts with a citation/test. Otherwise the first workflow will tell users to call a command that does not exist. -
The PR does not add focused test coverage despite adding a new adapter and tool ID. Please add at least adapter/registry coverage for
devin, plus a focused init/update test that proves.devin/workflows/opsx-*.mdare generated/refreshed and the tool is detected from.devin. That would have caught the earlier CodeRabbit wiring issue and will protect this adapter as the tool list keeps growing.
What I ran locally on head fd974e1:
pnpm install --frozen-lockfile(build passed via prepare)pnpm vitest run test/core/command-generation/adapters.test.ts test/core/command-generation/registry.test.ts test/core/init.test.ts test/core/update.test.ts test/core/available-tools.test.ts(219 tests passed)- Manual isolated
node bin/openspec.js init --tools devin --force, which generated the expected.devin/files but confirmed the body still contains/opsx:applystyle references.
Add test coverage for the Devin Desktop adapter including: - Command reference transformation from colon to hyphen syntax - YAML frontmatter escaping for special characters and implicit scalars - File path generation for workflows - Integration with available tools detection - Init and update command workflows
|
Hey @alfred-openspec, Thanks a lot for your review. I've addressed all of your comments and applied the requested changes. Could you please take another look at the PR when you have a chance? Thanks! |
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)
test/core/command-generation/adapters.test.ts (1)
747-779: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd cross-platform path test for Devin adapter.
The Devin adapter is missing from the cross-platform path verification tests. Major adapters (Claude, Cursor, Windsurf) have individual path.join verification tests in this section, but Devin does not.
As per coding guidelines, when touching path behavior, add coverage that would fail on Windows path separators.
🧪 Proposed test to add after line 763
it('Windsurf adapter uses path.join for paths', () => { const filePath = windsurfAdapter.getFilePath('test'); expect(filePath.split(path.sep)).toEqual(['.windsurf', 'workflows', 'opsx-test.md']); }); + + it('Devin adapter uses path.join for paths', () => { + const filePath = devinAdapter.getFilePath('test'); + expect(filePath.split(path.sep)).toEqual(['.devin', 'workflows', 'opsx-test.md']); + }); it('All adapters use path.join for paths', () => {🤖 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/core/command-generation/adapters.test.ts` around lines 747 - 779, The Devin adapter is missing from the cross-platform path tests; add a test that calls devinAdapter.getFilePath('test') and asserts the path is constructed via path.join semantics (e.g., split by path.sep yields the expected segments like ['.devin','commands','opsx-test.md'] or otherwise verify it contains path.sep), similar to the existing Claude/Cursor/Windsurf tests—locate the test block in adapters.test.ts and insert the new "Devin adapter uses path.join for paths" it() that references devinAdapter and its getFilePath method.Source: Coding guidelines
🤖 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 `@test/core/command-generation/adapters.test.ts`:
- Around line 747-779: The Devin adapter is missing from the cross-platform path
tests; add a test that calls devinAdapter.getFilePath('test') and asserts the
path is constructed via path.join semantics (e.g., split by path.sep yields the
expected segments like ['.devin','commands','opsx-test.md'] or otherwise verify
it contains path.sep), similar to the existing Claude/Cursor/Windsurf
tests—locate the test block in adapters.test.ts and insert the new "Devin
adapter uses path.join for paths" it() that references devinAdapter and its
getFilePath method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b243bd9-9b9a-40b1-92ba-482a3ace432a
📒 Files selected for processing (6)
src/core/command-generation/adapters/devin.tstest/core/available-tools.test.tstest/core/command-generation/adapters.test.tstest/core/command-generation/registry.test.tstest/core/init.test.tstest/core/update.test.ts
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-reviewed head cbb8301 after the follow-up commits. The blockers from my prior review are addressed: Devin workflow bodies now rewrite /opsx:* references to /opsx-*, .devin/workflows/ generation/update is covered, AI_TOOLS/registry/detection wiring is present, and the CodeRabbit path-test nit is covered.
Verified locally:
pnpm install --frozen-lockfilepnpm vitest run test/core/command-generation/adapters.test.ts test/core/command-generation/registry.test.ts test/core/init.test.ts test/core/update.test.ts test/core/available-tools.test.ts(232 passed)pnpm buildnode bin/openspec.js validate add-devin-desktop-support --strict- Manual
node bin/openspec.js init --tools devin --force <tmp>confirmed.devin/skills/*and.devin/workflows/opsx-*.mdare generated with no remaining/opsx:references.
|
Hi @TabishB and @alfred-openspec, Could this PR be included in the next release? Since Devin acquired Windsurf and dropped support for the old structure, our team needs this update to transition smoothly from OpenSpec/Windsurf to Devin. |
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
I want to include Devin Desktop support in the next release, but this rebased head needs two fixes before it is release-ready:
- Refresh the change deltas against the current canonical specs. On exact head
254fec2,node bin/openspec.js archive add-devin-desktop-support --yesaborts because the modifiedAI Tool Configurationblock omits the currentSelecting tools to configurescenario.validate --strictpasses, so please use the real archive path as the regression check. - Make the Devin skill path usable by Devin Local. Cognition's current docs say Devin Local does not support Workflows and recommends Skills instead; a clean
openspec init --tools devin --forcegenerates six.devin/skills/*files, but four retain/opsx:*workflow references and the getting-started hint prints/opsx:propose. Keep.devin/workflows/opsx-*.mdwith hyphen references for Cascade, but make Devin skill bodies and onboarding use/openspec-*, with focused tests for both surfaces.
The implementation builds, lints, and passes the full suite (2266/2266). Once those two blockers are fixed, re-request review and we can target the next release.
…t skills Addresses the two release blockers on this PR. Archive: the change's MODIFIED blocks were written against an older canonical `cli-init`, so `openspec archive add-devin-desktop-support` aborted rather than merging. The deltas are regenerated from the current canonical specs (cli-init `Skill Generation` + `Slash Command Generation`, cli-update `Slash Command Updates`, and a new `ai-tool-paths` delta for the `.devin` skillsDir), each restating every existing scenario so archive is purely additive. Invocation syntax: only Devin Desktop reads `.devin/workflows/`, so a `/opsx-*` workflow reference is dead text on Devin Local, which supports skills only. Devin now takes the skill-reference transformer, so skill bodies and the getting-started hint say `/openspec-*`. Workflow bodies keep hyphen references, applied by devinAdapter itself. The adapter also drops its private copy of escapeYamlValue / formatTagsArray in favor of the shared helpers main centralized in Fission-AI#1447, which quote unconditionally and escape control characters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…faces Follow-up from adversarial review of the previous commit. The devin special case in getTransformerForTool was unconditional, so under commands-only delivery — where `.devin/skills/` is deleted — the getting-started hint named `/openspec-propose`, a skill that is not on disk. Devin now takes the skill transformer only when skills are generated, and the hyphen form otherwise. The cli-init delta records the fallback, and a unit test pins all three delivery modes. Docs: `devin` was missing from the `--tools` list in docs/cli.md (which mirrors the list supported-tools.md already had) and from the command-syntax tables in docs/commands.md and docs/how-commands-work.md. The supported-tools row gains a footnote citing Cognition's docs for the `.windsurf/` -> `.devin/` move and the Devin Local workflow gap. Tests: init and update now assert both surfaces — workflows carry `/opsx-*`, skills carry `/openspec-*`, neither carries `/opsx:` — and update checks the seeded skill was actually refreshed. Adds the negative detection case. Drops three devin-only YAML assertions that duplicated, less rigorously, the registry-derived escaping matrix that now enrolls devin automatically. Also reverts an unrelated zcode export and lingma reorder that a merge resolution had pulled into adapters/index.ts. zcodeAdapter is registered but missing from that barrel on main; that is a pre-existing gap and belongs in its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
clay-good
changed the title
|
Addressing every open review comment. Pushed as @alfred-openspec (2026-07-27)1. Refresh the change deltas — archive aborted. Fixed. All deltas regenerated from the current canonical specs, each 2. Devin skill bodies and onboarding should use One wrinkle worth flagging: under commands-only delivery @coderabbitai
Also fixed, from an adversarial review pass
Full suite green: 3309/3309. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR changes =='
git diff --name-status origin/main...HEAD
echo '== Command-generation structure =='
ast-grep outline src/core/command-generation --items all --type function,variable 2>/dev/null || true
echo '== Devin-related implementation and tests =='
rg -n -C 3 --glob '*.ts' \
'(devinAdapter|toolId:\s*[\x27"]devin[\x27"]|getTransformerForTool|skill-reference|openspec-|opsx-|yaml)' \
src/core test/core
echo '== Tool config and documentation mentions =='
rg -n -C 2 '(Devin|devin|--tools)' \
src/core/config.ts docs/cli.md docs/commands.md docs/how-commands-work.md docs/supported-tools.md
echo '== Change delta headings and Devin-related requirements =='
rg -n -C 3 '(MODIFIED Requirements|REQUIREMENT:|Devin|devin|Slash Command Configuration|Slash Command Updates|ai-tool-paths)' \
openspec/changes/add-devin-desktop-support openspec/specs/cli-init openspec/specs/cli-update openspec/specs/ai-tool-pathsLength of output: 191730 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed files and commits =='
git diff --name-status origin/main...HEAD
git log --oneline origin/main..HEAD --max-count=20
echo '== Adapter, shared YAML helper, registry =='
for f in \
src/core/command-generation/adapters/devin.ts \
src/core/command-generation/yaml.ts \
src/core/command-generation/registry.ts \
src/core/command-generation/adapters/index.ts \
src/utils/command-references.ts
do
echo "--- $f"
cat -n "$f"
done
echo '== Focused test locations =='
rg -n -C 8 --glob '*.ts' \
'(Devin|devin|commands-only|skill-reference|implicit|077|\.inf|\.nan|carriage|negative)' \
test/core/init.test.ts \
test/core/update.test.ts \
test/core/command-references.test.ts \
test/core/available-tools.test.ts \
test/core/command-generation/adapters.test.ts \
test/core/command-generation/registry.test.ts
echo '== Exact requirement delta comparison =='
for spec in cli-init cli-update ai-tool-paths; do
echo "--- canonical $spec"
cat "openspec/specs/$spec/spec.md"
echo "--- delta $spec"
cat "openspec/changes/add-devin-desktop-support/specs/$spec/spec.md"
doneLength of output: 58083 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Devin transformer and tool validation tests =='
rg -n -C 12 \
'(getTransformerForTool|devin|commands-only|commands delivery|invalid.*tool|unknown.*tool|--tools)' \
test/utils/command-references.test.ts test/core/init.test.ts
echo '== YAML matrix registration and assertions =='
sed -n '1035,1175p' test/core/command-generation/adapters.test.ts
echo '== Complete canonical and delta specifications =='
for spec in cli-init cli-update ai-tool-paths; do
echo "--- canonical: openspec/specs/$spec/spec.md"
cat -n "openspec/specs/$spec/spec.md"
echo "--- delta: openspec/changes/add-devin-desktop-support/specs/$spec/spec.md"
cat -n "openspec/changes/add-devin-desktop-support/specs/$spec/spec.md"
done
echo '== Direct changes for reviewed implementation files =='
git diff --unified=5 origin/main...HEAD -- \
src/core/config.ts \
src/core/command-generation/adapters/devin.ts \
src/core/command-generation/yaml.ts \
src/utils/command-references.ts \
test/utils/command-references.test.ts \
test/core/init.test.ts \
test/core/update.test.tsLength of output: 84086
✏️ Learnings added
|
The profile-migration notice printed by both `init` and `update` hardcoded `/opsx:propose` for every adapter-backed tool. Devin registers no such command on any surface — its workflows answer to `/opsx-propose` and its skills to `/openspec-propose` — so an upgrading Devin user was told to run something that does not exist: Migrated: custom profile with 6 workflows New in this version: /opsx:propose. The reference now goes through getTransformerForTool, the same call init.ts already makes for the getting-started hint. Devin prints `/openspec-propose`; opencode and the other filename-invoked tools are corrected to `/opsx-propose` as a side effect; claude is unchanged. Also corrects two inherited false claims in the cli-update delta — Devin workflows carry no OpenSpec markers, and update writes every profile workflow rather than only refreshing files that already exist, which the PR's own test demonstrates. Qualifies the supported-tools footnote for commands-only delivery, and strips trailing whitespace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
CI note: |
The delta restates the whole 'Slash Command Updates' requirement, and its copy of the OpenCode scenario predated Fission-AI#1471 — archiving it would have quietly reverted the spec to calling the hyphen rewrite an OpenCode special case, the hand-maintained framing Fission-AI#1471 removed. Archive on a scratch copy is now purely additive. Also point tasks.md at the generator rather than the deleted transformToHyphenCommands, and enroll devin in the pure-formatter tripwire — it is the one adapter whose private body transform was just removed, so it is the likeliest to have it re-added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ration
Windsurf was rebranded to Devin Desktop on 2026-06-02 and its config
directory moved: `.devin/` is the preferred read+write location, `.windsurf/`
a legacy read-only fallback. Devin Local does not read `.windsurf/` at all,
so an existing Windsurf user's OpenSpec files are invisible to it.
Carrying `devin` as a second tool id alongside `windsurf` would list one
product twice and leave upgraders with two parallel installs — `openspec
update` even told them to create the second one ("Detected new tool: Devin
Desktop"). This follows the rename instead, as the repo already did for
Kimi CLI -> Kimi Code:
- `windsurf` is retired as a tool id; `devin` takes its place, with
`detectionPaths: ['.devin', '.windsurf']` so pre-rebrand projects are
still recognized. The Windsurf adapter is replaced, not duplicated.
- `TOOL_ID_ALIASES` keeps `--tools windsurf` resolving, so existing setup
scripts and CI keep working; they now configure `.devin/`.
- OpenSpec-managed skills (`openspec-*`) and command files (`opsx-*`) under
`.windsurf/` move to `.devin/`. The kimi migration handled skills only;
command files now move too, deriving the legacy path from the adapter's
own getFilePath rather than hard-coding a layout.
- The move is offered, not taken: nothing on disk distinguishes a user who
took the rebrand from one still on a pre-rebrand Windsurf build that reads
only `.windsurf/`. `openspec update` explains the rename and asks; --force
and non-interactive runs migrate; declining leaves every file untouched and
says what that costs. Files the user wrote are never moved.
Also gives Devin its own row in the authoritative invocation table — the
catch-all row claimed `/opsx-<id>` for both agents, which is wrong for Devin
Local.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adversarial pass found two ways the move destroyed files. Symlinked roots wiped the install. `ln -s .devin .windsurf` is a realistic way to straddle the rebrand, and it makes source and destination the same file — so the "destination exists, drop the legacy copy" branch deleted the only copy. Twelve generated files, gone, and not regenerated: the wipe happens before tool detection, so update then reported no configured tools. Both roots are now realpath'd and a self-move is skipped. User content inside an OpenSpec-managed path was deleted. The same branch rm -rf'd the whole legacy skill directory, taking a hand-written reference.md beside SKILL.md with it, and deleted a legacy command file even when the user had edited it. Now only SKILL.md is removed from a skill directory, and a command file is removed only when byte-identical to the one that survives — an edit is left where it is. Also: declining the move stranded the user. `update` then printed "No configured tools found. Run openspec init", which is wrong — the project is configured, just in the directory OpenSpec no longer writes. It now says so and how to resume. A closed stdin during the prompt aborted the whole update; it is treated as a decline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
clay-good
changed the title
|
@mehdishahdoost — thanks for this, and sorry it sat for a while. I've pushed to your branch to resolve the conflicts with Your original goal is intact and shipped: OpenSpec supports Devin Desktop, What changed and why. The review comment was that migration logic was needed. Digging into it, adding
So it now follows the rename rather than duplicating the tool — Three things worth flagging from the merge, since they're behavior changes to code you wrote:
Full details in the PR description. Happy to walk through any of it, and if you'd rather take a different approach on the rename, say so — it's your PR and the design call is worth discussing. |
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
One data-loss path remains on d94af8b: when .windsurf/skills/openspec-*/ contains a user file beside SKILL.md, migration renames the whole directory into .devin/, then commands-only delivery or a deselected workflow recursively removes it. A focused commands-only regression loses reference.md with ENOENT; please move only SKILL.md when sibling files exist and cover the commands-only/deselected cases, then I can approve.
alfred caught a data-loss path the earlier fix missed. When the destination did not yet exist, migration renamed the whole legacy skill directory into `.devin/` — carrying any file the user kept beside `SKILL.md` with it. That destination is a directory OpenSpec owns and removes on its own: under commands-only delivery, or for a workflow outside the active profile. So the move handed the user's file to a later rm and it vanished. Reproduced on `d94af8b`: with `delivery: commands`, a `reference.md` beside a legacy `SKILL.md` was gone after `openspec update`. Only `SKILL.md` crosses now, in both branches; anything else stays under the legacy root, and the legacy directory is still removed when the move leaves it empty. Regression tests cover the commands-only and deselected-workflow cases and both fail against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@alfred-openspec — confirmed and fixed in Reproduced first, on The mechanism is exactly as you described: when the destination did not yet exist, migration Fix: only Verified across all three delivery modes and the deselected-workflow case:
Three regression tests added covering the commands-only and deselected-workflow cases plus the clean-vacate case. I mutation-checked them: reverting to the whole-directory rename fails the two new data-loss tests, so they aren't vacuous. Spec delta and changeset updated to say "each skill's |
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 2a9c0e9. The user-file preservation blocker is fixed for both commands-only delivery and deselected workflows; build, lint, 1,527 focused tests, strict validation, a clean archive, all 36 archived specs, and hosted CI/security passed.
There was a problem hiding this comment.
Re-reviewed exact head 2a9c0e93. The migration now moves only generated SKILL.md files, preserves sibling user files under the legacy root, and covers commands-only and deselected-workflow cleanup; clean build, lint, focused tests, and strict change validation all pass.
…y treated A final adversarial pass found the two paths disagreeing. When both roots held the same file with different content, the command path compared bytes and kept the user's version; the skill path deleted it with no comparison — so one `openspec update` destroyed an edited SKILL.md while preserving an edited opsx-*.md in the same project. Both now share one `classifyManagedFile` rule: move when the destination is empty, drop the legacy copy only when byte-identical, otherwise leave it. Anything left behind is reported, so a user who customized a file knows two copies exist rather than discovering it later. Note on the other finding from that pass: OpenSpec regenerating or pruning the files it owns is long-standing behavior, not something this PR introduces. Verified against main — an edited SKILL.md under a deselected workflow, and an edited selected skill and command, are all destroyed by `openspec update` on 9a937cb too. No regression, so left alone here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
One reporting path is still dropped on 90eaee1: collectLegacyToolMigrations only returns a migration when moved > 0, so when every legacy file differs from its .devin counterpart, keptInPlace > 0 but update neither prompts nor emits keptInPlaceNotice, silently leaving two divergent copies. Please retain and report kept-only results with a focused all-edited regression, while avoiding an empty or misleading “Move …” prompt when nothing is actually movable.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
The preservation behavior is fixed, but the promised warning disappears when every candidate differs: collectLegacyToolMigrations() only returns a migration when skills.moved or commands.moved is nonzero, so an all-keep run never reaches keptInPlaceNotice(). Please include skills.kept || commands.kept in that predicate and assert the edited-skill/edited-command test prints the notice; the focused regression currently fails with both files preserved but no warning.
collectLegacyToolMigrations only returned a result when something moved, so a project where EVERY legacy file differs from its counterpart produced no output at all — two divergent copies and not a word about them. That is the one case where the report matters most, since it is entirely made of files the migration deliberately refused to touch. Kept-only results are retained now. Callers gate on hasMovableContent(), so a kept-only result reports what was left without offering to move nothing and without claiming a migration that did not happen. Also reworded the notice. A legacy file can differ because the user edited it or simply because an older OpenSpec generated it, so it no longer asserts an edit — it states that nothing was overwritten and leaves the user to compare the two copies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@alfred-openspec — confirmed and fixed in Reproduced first on Fix: Regression test added — the all-edited case, asserting the notice fires, that no One thing I changed beyond your ask. The notice used to say "your version was kept rather than overwritten", which asserts a user edit. A legacy file also differs when an older OpenSpec version generated it, so the wording now states only what's true — nothing was overwritten — and leaves the user to compare. Full suite 3365 passing, typecheck and lint clean, |
The kept-only regression asserted no line matched /Migrated\s*:/, which also matches OpenSpec's profile migration message, "Migrated: custom profile with N workflows". That line only prints when the global config has no profile yet — true on a fresh CI runner, false on a developer machine that has run OpenSpec before — so the test passed locally and failed on all three CI platforms. Now matched on the directory arrow, ".windsurf → .devin", which is specific to a migration report and unaffected by config state. Reproduced both ways with an empty XDG_CONFIG_HOME: the old assertion fails there, the new one passes, and the full suite is green under CI's XDG_CONFIG_HOME + VITEST_MAX_WORKERS=4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Follow-up on What broke. The kept-only regression asserted that no log line matched Root-caused rather than guessed. Fix: match on the directory arrow Then re-ran the whole suite under CI-equivalent conditions ( All 12 checks green on |
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-reviewed exact head c164701. Kept-only migrations now report divergent files without offering a no-op move or claiming a migration; build, lint, 1,529 focused tests, strict validation, a clean archive, all 36 archived specs, and hosted CI/security passed.
Thank you, @mehdishahdoost 👑This is approved and going in. Before it does, credit where it's due — and an honest account of what happened to your PR, because it changed a lot after you opened it. You found the problem, and you got the hard part right. Every new Devin user was being set up in a deprecated directory, and nobody had noticed. Two things in your PR were genuinely non-obvious and survived unchanged into the merged version:
What changed and why. The review asked for migration logic. Once we dug in, adding So it became a rename rather than an addition — And it took a while. The migration moves and deletes files in people's repos, so it got five review rounds and turned up four separate data-loss bugs before it was safe — including two that we introduced while fixing the previous one. None of that is a reflection on your PR; it's what that particular corner of the codebase demanded once we decided to touch existing users' files. Your adapter, your invocation analysis, and your docs research are all still in here. Sorry it sat as long as it did, and thanks for your patience through the rework and for the docs digging that made the whole thing possible. Genuinely good contribution — please send more. 🙏 |
3 participants
Status
Approved and ready to merge. This is @mehdishahdoost's PR, brought up to date with
mainand extended to cover the upgrade path for people who already use Windsurf. The original goal is unchanged and fully delivered: OpenSpec supports Devin Desktop. The scope grew for one reason, explained below.What was missing
OpenSpec supported Windsurf but not Devin Desktop. Windsurf was rebranded to Devin Desktop on June 2, 2026, and the rebrand moved the config directory:
.devin/is now the preferred read + write location and.windsurf/a legacy read-only fallback. Every Devin user runningopenspec initwas landing in the deprecated path.Why this PR changed shape
The original PR added
devinas a second tool alongsidewindsurf. Reviewing the upgrade path showed that leaves existing users worse off, not better:.devin/itself, so anyone who took the rebrand ends up with both directories. OpenSpec then said "Detected new tool: Devin Desktop. Run 'openspec init' to add it." It is not a new tool — it is the tool they already had, renamed. Following that advice produced 24 OpenSpec files across two directories for one editor, and two contradictory getting-started lines.openspec updateand stayed on the deprecated path forever..windsurf/at all. So an existing Windsurf user's OpenSpec files are completely invisible to that agent. The "legacy fallback" only helps on Devin Desktop.So this follows the rename instead of duplicating the tool — exactly what OpenSpec already did for Kimi CLI → Kimi Code (
.kimi→.kimi-code).What it does
For a new Devin user:
openspec init --tools devinwrites.devin/workflows/opsx-<id>.mdand.devin/skills/openspec-*/SKILL.md. Nothing else to know.For an existing Windsurf user:
openspec updateexplains the rebrand and offers to move your OpenSpec files to.devin/..windsurf/, and nothing on disk tells that user apart from someone who took the rebrand — so the move is offered, never assumed.--forceand non-interactive runs (CI) take the move without asking.SKILL.mdand commands namedopsx-*. A hand-written Cascade workflow, a reference file you keep beside aSKILL.md, and.devin/rules/are all left exactly where they are.--tools windsurfstill works. A retired-id alias resolves it todevin, so existing setup scripts and CI don't break. It now configures.devin/.Two agents, two syntaxes. Devin Desktop reads workflows; Devin Local does not. So workflow bodies say
/opsx-<id>(the name a workflow filename registers) while skill bodies and the getting-started hint say/openspec-*, which works on both agents. Under commands-only delivery no skills exist, so both fall back to/opsx-*.Proof it works
Full suite: 3365/3365 passing, run under CI-equivalent conditions (empty
XDG_CONFIG_HOME+VITEST_MAX_WORKERS=4). Typecheck andeslint src/clean.openspec validate add-devin-desktop-support --strictpasses, and archiving on a scratch copy removes exactly the six intended retired-Windsurf lines.30 behaviors verified by running the CLI against the built binary, not just tests:
.windsurfupgrade; repeatupdateis idempotent;--tools windsurfandexperimental --tool windsurfboth write.devin/onlySKILL.mdunder commands-only delivery and under a deselected workflow.devinwins; identical duplicate removed; edited skill and edited command both kept, symmetrically; all-divergent case reported with no phantom migration line/opsx:leakage in.devin/; workflows/opsx-*, skills/openspec-*; correct hint in all three delivery modes.kimi→.kimi-codeintact with user files preserved; multi-tool init clean; IDE-written.devin/rules/preserved; no bogus "new tool" noticeThe migration got five review rounds because it deletes files. Four data-loss defects were found and fixed along the way — symlinked roots deleting the only copy;
rm -rfon a skill directory taking a user's notes; a whole-directory rename carrying a user file into a directory OpenSpec later prunes; and the skill path deleting an editedSKILL.mdwhile the command path compared bytes. Each fix has a regression test that was mutation-checked (it fails against the previous code).Notes
windsurftool id is retired. This is the one product decision here, and the reason the diff is larger than the original. The alias means nothing breaks, but OpenSpec now lists this editor once, as Devin Desktop.main— an editedSKILL.mdunder a deselected workflow is destroyed byopenspec updateon9a937cbtoo. Out of scope here.RETIRED —. They can't be renamed: OpenSpec's own archive engine refuses to drop scenarios from aMODIFIEDblock, which is the correct guard.mainhad to adapt to fix(adapters): reference slash commands by the names each tool registers #1471, which replaced the per-tool list ingetTransformerForToolwith a derived invocation and moved command-body rewriting into the generator. The Devin adapter is now a pure formatter like every other one..devin/also coversrules/andplans/. OpenSpec writes neither; untouched.