[codex] Add workspace change planning workflow#1089
Merged
Conversation
Contributor
|
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 workspace-planning schema/templates/specs; planning-home resolver; threads planningHome through artifact-graph status/instructions; adds workspace setup/update with workspace-local skill generation and drift detection; updates CLI (new-change flags, update redirect, config profile guidance); updates workflow templates to use resolved paths and workspace guards; and adds tests/docs. ChangesWorkspace planning and skills integration
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI
participant PlanningHome
participant WorkspaceSkills
participant InstructionLoader
User->>CLI: openspec update
CLI->>PlanningHome: resolveCurrentPlanningHomeSync()
alt workspace root found
CLI->>WorkspaceSkills: runWorkspaceUpdateForRoot(root, options)
WorkspaceSkills-->>User: workspace_skills report (added/removed/refreshed/skipped/failed)
else repo or no workspace
CLI-->>User: proceed with repo update
end
User->>CLI: openspec status --change <id> --json
CLI->>PlanningHome: resolveCurrentPlanningHomeSync()
CLI->>InstructionLoader: loadChangeContext({ planningHome, changeDir })
InstructionLoader-->>User: { planningHome, changeRoot, artifactPaths, actionContext, nextSteps }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 3
🧹 Nitpick comments (4)
test/core/planning-home.test.ts (1)
10-28: ⚡ Quick winConsider adding a POSIX path test case for cross-platform coverage.
The test currently only validates Windows-style paths (
D:\repos\...). Adding a parallel test case with POSIX paths would ensure the functions work correctly on all platforms.♻️ Suggested additional test case
+ it('builds workspace change paths with POSIX path style', () => { + const workspacePlanningHome: PlanningHome = { + kind: 'workspace', + root: '/repos/platform-workspace', + changesDir: '/repos/platform-workspace/changes', + defaultSchema: 'workspace-planning', + workspace: { + name: 'platform', + links: ['api', 'web'], + }, + }; + + expect(getChangeDir(workspacePlanningHome, 'cross-repo-login')).toBe( + '/repos/platform-workspace/changes/cross-repo-login' + ); + expect(formatChangeLocation(workspacePlanningHome, 'cross-repo-login')).toBe( + 'changes/cross-repo-login' + ); + });🤖 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/planning-home.test.ts` around lines 10 - 28, The test only covers Windows-style paths; add a parallel POSIX-style test case to validate cross-platform behavior by creating a PlanningHome object with POSIX paths (e.g., root: '/home/repos/platform-workspace', changesDir: '/home/repos/platform-workspace/changes') and assert getChangeDir(workspacePlanningHomePosix, 'cross-repo-login') returns '/home/repos/platform-workspace/changes/cross-repo-login' and formatChangeLocation(workspacePlanningHomePosix, 'cross-repo-login') returns 'changes/cross-repo-login'; update or add a new it(...) that mirrors the existing Windows test but uses the PlanningHome type and the getChangeDir and formatChangeLocation functions for POSIX validation.src/core/workspace/skills.ts (1)
348-349: ⚡ Quick winHardcoded tool IDs for command transformation.
The logic hardcodes
'opencode'and'pi'as tool IDs requiring hyphen command transformation. If new tools with similar requirements are added, this code will need manual updates.♻️ Consider adding a tool property
Add a
requiresHyphenTransform?: booleanproperty to the tool definition inAI_TOOLSand check it here:- const transformer = - tool.value === 'opencode' || tool.value === 'pi' ? transformToHyphenCommands : undefined; + const transformer = tool.requiresHyphenTransform ? transformToHyphenCommands : undefined;This would centralize tool-specific configuration and make it easier to maintain.
Also applies to: 460-461
🤖 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/core/workspace/skills.ts` around lines 348 - 349, The code currently sets transformer based on hardcoded tool IDs ('opencode' and 'pi') using tool.value and transformToHyphenCommands; instead add a tool-level boolean (e.g., requiresHyphenTransform?: boolean) to each entry in AI_TOOLS and update the assignment to check tool.requiresHyphenTransform (or AI_TOOLS[tool.value].requiresHyphenTransform) instead of comparing strings; apply the same change to the other occurrence that currently checks 'opencode'/'pi' so future tools can enable the hyphen transform via configuration rather than code changes.schemas/workspace-planning/templates/spec.md (1)
1-10: 💤 Low valueConsider adding examples for all delta operation types.
The schema instruction (lines 33-38 in
schema.yaml) defines four delta operation types: ADDED, MODIFIED, REMOVED, and RENAMED requirements. The template currently shows only the ADDED section. While the schema instructions provide text guidance for the other types, including template examples for MODIFIED, REMOVED, and RENAMED sections would make the template more helpful and reduce ambiguity for users creating workspace specs.🤖 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 `@schemas/workspace-planning/templates/spec.md` around lines 1 - 10, The template only includes an "ADDED Requirements" example; add analogous example sections for MODIFIED, REMOVED, and RENAMED requirement deltas to match the schema.yaml delta types. For each new section (e.g., "MODIFIED Requirements", "REMOVED Requirements", "RENAMED Requirements") provide a short requirement header (like "Requirement: <workspace requirement name>") and a Scenario block using the GIVEN/WHEN/THEN format similar to the ADDED example so authors have concrete templates for each delta operation type.openspec/changes/workspace-change-planning/specs/workspace-links/spec.md (1)
1-164: 💤 Low valueConsider renaming this spec file to better reflect its content.
The file is named
workspace-links/spec.mdbut the requirements focus on workspace agent skills installation and update workflows, not workspace links as a concept. While the scenarios reference linked repos/folders, the primary subject matter is agent skill management during setup and update operations. A name likeworkspace-skills/spec.md,workspace-setup/spec.md, orcli-workspace-skills/spec.mdwould more accurately reflect the content and make the spec easier to locate.🤖 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/workspace-change-planning/specs/workspace-links/spec.md` around lines 1 - 164, This spec file's name (workspace-links/spec.md) doesn't match its content (workspace agent skill installation/update), so rename the file to a clearer name (e.g., workspace-skills/spec.md or cli-workspace-skills/spec.md) and update any references to it in docs or test harnesses; ensure the new name reflects the primary subject (agent skill setup/update) while keeping the existing content and preserving links to any mentions of "linked repos/folders" within the spec.
🤖 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
`@openspec/changes/workspace-change-planning/review-candidate-requirements.html`:
- Around line 729-742: The template assigns to card.innerHTML and injects
s.userComment into a textarea via string replacement, which allows XSS if the
comment contains HTML like </textarea>; fix by building the DOM nodes
programmatically instead of using innerHTML: create the container elements (the
sug-head, sug-text, sug-actions, textarea with class "sug-comment") with
document.createElement, set attributes and classes (including data-comment-for
using s.id), set the textarea's value or textContent to s.userComment (not via
string concatenation), and append the pieces to card so content is safely
escaped; ensure the approve/reject/reset buttons and their is-active logic
(based on s.status) and visibility logic (based on state.activeId === s.id) are
preserved when converting the innerHTML block.
In `@src/cli/index.ts`:
- Around line 168-171: The redirect-to-workspace code is dropping the parsed
--force option by calling runWorkspaceUpdateForRoot(workspaceRoot, {}); change
this to propagate the parsed force flag (e.g., pass { force } or { force:
options.force } depending on your option variable) so runWorkspaceUpdateForRoot
receives the same force semantics as the top-level command; update the call in
the branch that uses workspaceRoot (and ensure the option variable name in
scope—such as force or options.force—is referenced).
In `@src/core/workspace/skills.ts`:
- Around line 101-107: The arraysEqual function currently does an
order-sensitive element-by-element comparison causing false positives when
workflow ID order changes; update arraysEqual (and any callers like
last_applied_workflow_ids checks) to perform an order-insensitive set
comparison: treat undefined as empty, if lengths differ return false, convert
both arrays to Sets and ensure every element in one Set exists in the other (and
sizes match) so identical ID sets in different orders are considered equal.
---
Nitpick comments:
In `@openspec/changes/workspace-change-planning/specs/workspace-links/spec.md`:
- Around line 1-164: This spec file's name (workspace-links/spec.md) doesn't
match its content (workspace agent skill installation/update), so rename the
file to a clearer name (e.g., workspace-skills/spec.md or
cli-workspace-skills/spec.md) and update any references to it in docs or test
harnesses; ensure the new name reflects the primary subject (agent skill
setup/update) while keeping the existing content and preserving links to any
mentions of "linked repos/folders" within the spec.
In `@schemas/workspace-planning/templates/spec.md`:
- Around line 1-10: The template only includes an "ADDED Requirements" example;
add analogous example sections for MODIFIED, REMOVED, and RENAMED requirement
deltas to match the schema.yaml delta types. For each new section (e.g.,
"MODIFIED Requirements", "REMOVED Requirements", "RENAMED Requirements") provide
a short requirement header (like "Requirement: <workspace requirement name>")
and a Scenario block using the GIVEN/WHEN/THEN format similar to the ADDED
example so authors have concrete templates for each delta operation type.
In `@src/core/workspace/skills.ts`:
- Around line 348-349: The code currently sets transformer based on hardcoded
tool IDs ('opencode' and 'pi') using tool.value and transformToHyphenCommands;
instead add a tool-level boolean (e.g., requiresHyphenTransform?: boolean) to
each entry in AI_TOOLS and update the assignment to check
tool.requiresHyphenTransform (or AI_TOOLS[tool.value].requiresHyphenTransform)
instead of comparing strings; apply the same change to the other occurrence that
currently checks 'opencode'/'pi' so future tools can enable the hyphen transform
via configuration rather than code changes.
In `@test/core/planning-home.test.ts`:
- Around line 10-28: The test only covers Windows-style paths; add a parallel
POSIX-style test case to validate cross-platform behavior by creating a
PlanningHome object with POSIX paths (e.g., root:
'/home/repos/platform-workspace', changesDir:
'/home/repos/platform-workspace/changes') and assert
getChangeDir(workspacePlanningHomePosix, 'cross-repo-login') returns
'/home/repos/platform-workspace/changes/cross-repo-login' and
formatChangeLocation(workspacePlanningHomePosix, 'cross-repo-login') returns
'changes/cross-repo-login'; update or add a new it(...) that mirrors the
existing Windows test but uses the PlanningHome type and the getChangeDir and
formatChangeLocation functions for POSIX validation.
🪄 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: 77aa2c5c-4555-4464-b8f2-bbe4deade43d
📒 Files selected for processing (64)
docs/cli.mddocs/concepts.mdopenspec/changes/workspace-change-planning/candidate-requirements.mdopenspec/changes/workspace-change-planning/deep-dive-requirements.htmlopenspec/changes/workspace-change-planning/design.mdopenspec/changes/workspace-change-planning/exploration-summary.mdopenspec/changes/workspace-change-planning/manual-acceptance.mdopenspec/changes/workspace-change-planning/prd.mdopenspec/changes/workspace-change-planning/proposal.mdopenspec/changes/workspace-change-planning/review-candidate-requirements.htmlopenspec/changes/workspace-change-planning/specs/artifact-graph/spec.mdopenspec/changes/workspace-change-planning/specs/change-creation/spec.mdopenspec/changes/workspace-change-planning/specs/cli-artifact-workflow/spec.mdopenspec/changes/workspace-change-planning/specs/cli-config/spec.mdopenspec/changes/workspace-change-planning/specs/cli-update/spec.mdopenspec/changes/workspace-change-planning/specs/openspec-conventions/spec.mdopenspec/changes/workspace-change-planning/specs/schema-resolution/spec.mdopenspec/changes/workspace-change-planning/specs/workspace-change-planning/spec.mdopenspec/changes/workspace-change-planning/specs/workspace-links/spec.mdopenspec/changes/workspace-change-planning/tasks.mdschemas/workspace-planning/schema.yamlschemas/workspace-planning/templates/design.mdschemas/workspace-planning/templates/proposal.mdschemas/workspace-planning/templates/spec.mdschemas/workspace-planning/templates/tasks.mdsrc/cli/index.tssrc/commands/config.tssrc/commands/workflow/instructions.tssrc/commands/workflow/new-change.tssrc/commands/workflow/shared.tssrc/commands/workflow/status.tssrc/commands/workspace.tssrc/commands/workspace/operations.tssrc/commands/workspace/selection.tssrc/commands/workspace/types.tssrc/core/artifact-graph/index.tssrc/core/artifact-graph/instruction-loader.tssrc/core/artifact-graph/types.tssrc/core/completions/command-registry.tssrc/core/index.tssrc/core/planning-home.tssrc/core/templates/workflows/apply-change.tssrc/core/templates/workflows/archive-change.tssrc/core/templates/workflows/bulk-archive-change.tssrc/core/templates/workflows/continue-change.tssrc/core/templates/workflows/explore.tssrc/core/templates/workflows/ff-change.tssrc/core/templates/workflows/new-change.tssrc/core/templates/workflows/onboard.tssrc/core/templates/workflows/propose.tssrc/core/templates/workflows/sync-specs.tssrc/core/templates/workflows/verify-change.tssrc/core/workspace/foundation.tssrc/core/workspace/index.tssrc/core/workspace/skills.tssrc/utils/change-metadata.tssrc/utils/change-utils.tstest/commands/artifact-workflow.test.tstest/commands/config-profile.test.tstest/commands/workspace.interactive.test.tstest/commands/workspace.test.tstest/core/planning-home.test.tstest/core/templates/skill-templates-parity.test.tstest/core/workspace/skills.test.ts
Comment on lines
+101
to
+107
| function arraysEqual(left: readonly string[] | undefined, right: readonly string[]): boolean { | ||
| if (!left || left.length !== right.length) { | ||
| return false; | ||
| } | ||
|
|
||
| return left.every((value, index) => value === right[index]); | ||
| } |
Contributor
There was a problem hiding this comment.
Potential issue: Array comparison doesn't check element order.
The arraysEqual helper compares arrays element-by-element using index position. If the workflow IDs are stored in different orders between last_applied_workflow_ids and the current profile, this will report drift even when the sets are identical. This could cause unnecessary skill regeneration.
🔧 Proposed fix to compare sets instead of ordered arrays
-function arraysEqual(left: readonly string[] | undefined, right: readonly string[]): boolean {
- if (!left || left.length !== right.length) {
- return false;
- }
-
- return left.every((value, index) => value === right[index]);
+function arraysEqual(left: readonly string[] | undefined, right: readonly string[]): boolean {
+ if (!left || left.length !== right.length) {
+ return false;
+ }
+
+ const leftSet = new Set(left);
+ const rightSet = new Set(right);
+ return left.every((value) => rightSet.has(value)) && right.every((value) => leftSet.has(value));
}🤖 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/core/workspace/skills.ts` around lines 101 - 107, The arraysEqual
function currently does an order-sensitive element-by-element comparison causing
false positives when workflow ID order changes; update arraysEqual (and any
callers like last_applied_workflow_ids checks) to perform an order-insensitive
set comparison: treat undefined as empty, if lengths differ return false,
convert both arrays to Sets and ensure every element in one Set exists in the
other (and sizes match) so identical ID sets in different orders are considered
equal.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@openspec/specs/schema-resolution/spec.md`:
- Around line 185-190: The spec currently has conflicting SHALLs about default
schema precedence: the "Workspace default schema for new changes" scenario
states omitted --schema defaults to "workspace-planning", while the earlier
"default from config" text (lines 95-103) says omitted --schema is taken from
config; update the specification to define a single, unambiguous precedence rule
for default schema resolution (e.g., "When --schema is omitted, workspace-level
defaults (workspace-planning) override any project or global config values" or
the opposite if intended), and apply that rule explicitly in both the "Workspace
default schema for new changes" scenario and the earlier default-from-config
section so both reference the same precedence behavior.
In `@openspec/specs/workspace-links/spec.md`:
- Line 443: Update the spec language that currently says "OpenSpec-managed
workflow skill directories" (lines referencing removal at lines 443, 450,
459-462) to explicitly define how OpenSpec identifies managed directories:
specify one or more allowed mechanisms (e.g., a required naming convention such
as directories prefixed with ".openspec-", the presence of a marker file like
".openspec-managed" inside the directory, or recording created directories in a
metadata registry), mandate the preferred/default mechanism, and add wording
that implementations MUST only delete directories matching the chosen
identification method to avoid removing user-created content; also add a short
example sentence and note where this mechanism applies (the removal rules at the
referenced lines).
🪄 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: c46ffcb7-3f4a-4850-9599-efa2afe32580
📒 Files selected for processing (31)
WORKSPACE_REIMPLEMENTATION_START_HERE.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/design.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/proposal.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/artifact-graph/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/change-creation/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/cli-artifact-workflow/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/cli-config/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/cli-update/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/openspec-conventions/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/schema-resolution/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/workspace-change-planning/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/specs/workspace-links/spec.mdopenspec/changes/archive/2026-05-14-workspace-change-planning/tasks.mdopenspec/changes/workspace-agent-guidance/.openspec.yamlopenspec/changes/workspace-agent-guidance/design.mdopenspec/changes/workspace-agent-guidance/proposal.mdopenspec/changes/workspace-agent-guidance/specs/change-creation/spec.mdopenspec/changes/workspace-agent-guidance/specs/cli-artifact-workflow/spec.mdopenspec/changes/workspace-agent-guidance/specs/workspace-links/spec.mdopenspec/changes/workspace-agent-guidance/tasks.mdopenspec/changes/workspace-reimplementation-roadmap/README.mdopenspec/changes/workspace-reimplementation-roadmap/proposal.mdopenspec/specs/artifact-graph/spec.mdopenspec/specs/change-creation/spec.mdopenspec/specs/cli-artifact-workflow/spec.mdopenspec/specs/cli-config/spec.mdopenspec/specs/cli-update/spec.mdopenspec/specs/openspec-conventions/spec.mdopenspec/specs/schema-resolution/spec.mdopenspec/specs/workspace-change-planning/spec.mdopenspec/specs/workspace-links/spec.md
✅ Files skipped from review due to trivial changes (14)
- openspec/changes/workspace-agent-guidance/.openspec.yaml
- openspec/specs/cli-update/spec.md
- openspec/changes/workspace-agent-guidance/tasks.md
- openspec/specs/change-creation/spec.md
- openspec/specs/workspace-change-planning/spec.md
- openspec/specs/openspec-conventions/spec.md
- openspec/changes/archive/2026-05-14-workspace-change-planning/specs/cli-update/spec.md
- openspec/changes/archive/2026-05-14-workspace-change-planning/specs/schema-resolution/spec.md
- openspec/changes/archive/2026-05-14-workspace-change-planning/proposal.md
- openspec/changes/archive/2026-05-14-workspace-change-planning/specs/change-creation/spec.md
- openspec/changes/workspace-agent-guidance/proposal.md
- openspec/changes/workspace-reimplementation-roadmap/README.md
- openspec/changes/archive/2026-05-14-workspace-change-planning/specs/cli-config/spec.md
- openspec/changes/archive/2026-05-14-workspace-change-planning/specs/workspace-change-planning/spec.md
alfred-openspec
approved these changes
May 14, 2026
Collaborator
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Approved. CI is green, PR is mergeable, and the workspace planning/update changes look ready to land.
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
openspec update, unmanaged skill preservation, failed skill-state recording, and template whitespace.Impact
This gives workspace users a cleaner path for planning and implementing change work with generated workflow skills that match the workspace context. It also tightens update safety so local user-authored skill directories are preserved and failed updates are not recorded as successfully applied.
Validation
pnpm exec eslint src/commands/workspace.ts src/commands/workspace/selection.ts src/core/workspace/skills.ts src/cli/index.tsgit diff --checkpnpm run buildpnpm exec vitest run test/commands/workspace.test.ts test/core/workspace/skills.test.tspnpm testnode dist/cli/index.js validate workspace-change-planning --strictSummary by CodeRabbit
New Features
openspec workspace updateand--toolsforworkspace setup; workspace skill selection is persisted and can be refreshed (JSON output & exit codes supported).openspec new changeaccepts--goaland--areasfor workspace-scoped planning; workspace-scoped changes use a new workspace-planning schema.Improvements
Documentation