[codex] Include sync in core workflow defaults#1030
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR makes Changes
Sequence Diagram(s)(Skipped — changes are code/docs/tests around workflow membership and migration guidance; no new multi-component control flow requiring a sequence diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/core/update.test.ts (1)
1430-1463: ⚡ Quick winAdd an explicit “no config mutation” assertion in this new scenario.
This test already verifies messaging/artifacts well; adding a direct check that
saveGlobalConfigis not called would lock in the non-mutating contract from the PR objective.✅ Small test hardening
it('should suggest core preset when custom profile preserves the old core workflow set', async () => { + const { saveGlobalConfig } = await import('../../src/core/global-config.js'); setMockConfig({ featureFlags: {}, profile: 'custom', delivery: 'both', workflows: ['propose', 'explore', 'apply', 'archive'], @@ path.join(testDir, '.claude', 'commands', 'opsx', 'sync.md') )).toBe(false); + expect(saveGlobalConfig).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/update.test.ts` around lines 1430 - 1463, Add an explicit assertion that ConfigUtils.saveGlobalConfig is not called in this test: before calling updateCommand.execute(testDir) spy on ConfigUtils.saveGlobalConfig (e.g., const saveSpy = vi.spyOn(ConfigUtils, 'saveGlobalConfig').mockResolvedValue(undefined)), run updateCommand.execute(...), then assert expect(saveSpy).not.toHaveBeenCalled(); finally restore the spy (saveSpy.mockRestore()) alongside consoleSpy.mockRestore(); place these lines in the same test that uses InitCommand and updateCommand so the non-mutating contract is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/migration-guide.md`:
- Line 11: Update the command listings so the “default core” and “expanded
workflow” sections are consistent: ensure the default core includes
/opsx:propose, /opsx:apply, /opsx:sync, /opsx:archive and also include
/opsx:explore (which was omitted), and make the expanded/optional list reflect
only additional commands beyond those core ones; search for occurrences of
/openspec:proposal, /openspec:apply, /openspec:archive and /opsx:propose,
/opsx:apply, /opsx:sync, /opsx:archive, /opsx:explore (including the other
occurrence mentioned in the comment) and replace or reconcile them so every
command table and sentence consistently shows sync in core and explore included
where appropriate.
---
Nitpick comments:
In `@test/core/update.test.ts`:
- Around line 1430-1463: Add an explicit assertion that
ConfigUtils.saveGlobalConfig is not called in this test: before calling
updateCommand.execute(testDir) spy on ConfigUtils.saveGlobalConfig (e.g., const
saveSpy = vi.spyOn(ConfigUtils,
'saveGlobalConfig').mockResolvedValue(undefined)), run
updateCommand.execute(...), then assert expect(saveSpy).not.toHaveBeenCalled();
finally restore the spy (saveSpy.mockRestore()) alongside
consoleSpy.mockRestore(); place these lines in the same test that uses
InitCommand and updateCommand so the non-mutating contract is enforced.
🪄 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: 326a0c34-bc6c-4b18-b40c-883ab2450ab2
📒 Files selected for processing (16)
.changeset/sync-default-core.mdREADME.mddocs/cli.mddocs/commands.mddocs/getting-started.mddocs/migration-guide.mddocs/opsx.mddocs/supported-tools.mdsrc/core/profiles.tssrc/core/update.tstest/commands/config-profile.test.tstest/commands/config.test.tstest/core/init.test.tstest/core/profile-sync-drift.test.tstest/core/profiles.test.tstest/core/update.test.ts
| | Aspect | Legacy | OPSX | | ||
| |--------|--------|------| | ||
| | **Commands** | `/openspec:proposal`, `/openspec:apply`, `/openspec:archive` | Default: `/opsx:propose`, `/opsx:apply`, `/opsx:archive` (expanded workflow commands optional) | | ||
| | **Commands** | `/openspec:proposal`, `/openspec:apply`, `/openspec:archive` | Default: `/opsx:propose`, `/opsx:apply`, `/opsx:sync`, `/opsx:archive` (expanded workflow commands optional) | |
Contributor
There was a problem hiding this comment.
Core command listings are now inconsistent within this doc.
After moving sync into default core, some command lists still reflect the old split (and Line 11 omits explore). Please align all “default core” and “expanded” sections in this file to avoid user confusion.
📝 Suggested doc alignment
-| **Commands** | `/openspec:proposal`, `/openspec:apply`, `/openspec:archive` | Default: `/opsx:propose`, `/opsx:apply`, `/opsx:sync`, `/opsx:archive` (expanded workflow commands optional) |
+| **Commands** | `/openspec:proposal`, `/openspec:apply`, `/openspec:archive` | Default: `/opsx:propose`, `/opsx:explore`, `/opsx:apply`, `/opsx:sync`, `/opsx:archive` (expanded workflow commands optional) | **Default (`core` profile):**
| Command | Purpose |
|---------|---------|
| `/opsx:propose` | Create a change and generate planning artifacts in one step |
| `/opsx:explore` | Think through ideas with no structure |
| `/opsx:apply` | Implement tasks from tasks.md |
+| `/opsx:sync` | Preview/spec-merge without archiving |
| `/opsx:archive` | Finalize and archive the change |
**Expanded workflow (custom selection):**
...
-| `/opsx:sync` | Preview/spec-merge without archiving |Also applies to: 87-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/migration-guide.md` at line 11, Update the command listings so the
“default core” and “expanded workflow” sections are consistent: ensure the
default core includes /opsx:propose, /opsx:apply, /opsx:sync, /opsx:archive and
also include /opsx:explore (which was omitted), and make the expanded/optional
list reflect only additional commands beyond those core ones; search for
occurrences of /openspec:proposal, /openspec:apply, /openspec:archive and
/opsx:propose, /opsx:apply, /opsx:sync, /opsx:archive, /opsx:explore (including
the other occurrence mentioned in the comment) and replace or reconcile them so
every command table and sentence consistently shows sync in core and explore
included where appropriate.
Contributor
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)
docs/workflows.md (1)
444-444:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the profile designation for
/opsx:syncin the reference table.The "When to Use" column currently says "Expanded mode, optional" for
/opsx:sync, but this is now inconsistent with the changes in this PR. Since sync is now part of the defaultcoreprofile (as documented in lines 36-40), the reference table should reflect this.📝 Suggested update
-| `/opsx:sync` | Merge delta specs | Expanded mode, optional | +| `/opsx:sync` | Merge delta specs | Core profile, optional |Or simply:
-| `/opsx:sync` | Merge delta specs | Expanded mode, optional | +| `/opsx:sync` | Merge delta specs | Optional (archive prompts if needed) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/workflows.md` at line 444, Update the reference table entry for `/opsx:sync` to reflect that sync is now part of the default core profile: replace the "When to Use" text "Expanded mode, optional" with wording that indicates it belongs to the core profile (e.g., "core profile (default)") so the table matches the PR's documentation changes; look for the table row containing the symbol `/opsx:sync` in the workflows reference and update that cell accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/workflows.md`:
- Line 444: Update the reference table entry for `/opsx:sync` to reflect that
sync is now part of the default core profile: replace the "When to Use" text
"Expanded mode, optional" with wording that indicates it belongs to the core
profile (e.g., "core profile (default)") so the table matches the PR's
documentation changes; look for the table row containing the symbol `/opsx:sync`
in the workflows reference and update that cell accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3afe425f-9312-4002-8cb0-c562c8c81034
📒 Files selected for processing (1)
docs/workflows.md
alfred-openspec
approved these changes
May 1, 2026
alfred-openspec
left a comment
alfred-openspec
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Checks are green and the core-profile sync default update looks scoped to profiles, update messaging, docs, tests, and changeset.
kaze-droid
pushed a commit
to kaze-droid/OpenSpec
that referenced
this pull request
May 10, 2026
* Include sync in core workflow defaults * Add old core custom profile sync hint * Update workflows sync default docs
kaze-droid
pushed a commit
to kaze-droid/OpenSpec
that referenced
this pull request
May 11, 2026
* Include sync in core workflow defaults * Add old core custom profile sync hint * Update workflows sync default docs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
syncto the defaultcoreworkflow profile so new installs generate/opsx:syncskills and commands by default.This also updates profile/init/update/config tests, user docs, and adds a changeset for the behavior change.
Existing Users
This PR does not silently mutate custom profiles.
coregetsyncthe next time they runopenspec update, becausecoreresolves throughCORE_WORKFLOWS.customkeep their configured workflow list.openspec updatesees a custom workflow set matching the old core defaults (propose,explore,apply,archive), it prints a non-blocking note explaining thatcorenow includessyncand tells users to runopenspec config profile corefollowed byopenspec updateto opt in.Validation
pnpm buildpnpm exec vitest run test/core/update.test.tspnpm lintpnpm testgit diff --checkSummary by CodeRabbit
New Features
Documentation
Tests