[codex] Polish beta context workspace flow#1136
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThis PR standardizes opener ids to ChangesWorkspace and Initiative Beta Features
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/commands/workspace/open.ts (1)
57-60: ⚡ Quick winRemove the unnecessary
opener.id as stringcast (and consider dropping unreachable'codex'check)In
src/commands/workspace/open.ts,WorkspacePreferredOpener.idis already typed asWorkspaceAgentOpenerId | WorkspaceEditorOpenerId, soconst openerId = opener.id as stringdoesn’t add type safety. Also, legacy opener value'codex'is normalized to'codex-cli'during workspace parsing, so(openerId === 'codex')is likely redundant.🤖 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/workspace/open.ts` around lines 57 - 60, The isCodexCliOpener function casts opener.id to string unnecessarily and checks a legacy 'codex' value; remove the cast by using opener.id directly (typed as WorkspaceAgentOpenerId | WorkspaceEditorOpenerId) and simplify the condition to only check for the normalized value 'codex-cli' when opener.kind === 'agent' (i.e., in isCodexCliOpener use opener.id === 'codex-cli' and drop the 'codex' branch); reference: function isCodexCliOpener and type WorkspacePreferredOpener / WorkspaceAgentOpenerId.docs/workspaces-beta/user-guide.md (1)
36-38: 💤 Low valueOptional: Consider varying the phrasing to improve readability.
The phrase "when you want to" appears twice in close proximity. While not incorrect, varying the language would improve flow.
Suggestion:
-Use `openspec workspace open --initiative team-context/billing-launch --editor` -when you want to skip the picker. Use `--agent codex-cli`, `--agent claude`, or -`--agent github-copilot` instead of `--editor` when you want to open an agent -directly. +Use `openspec workspace open --initiative team-context/billing-launch --editor` +to skip the picker. Use `--agent codex-cli`, `--agent claude`, or +`--agent github-copilot` instead of `--editor` to open an agent directly.🤖 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 `@docs/workspaces-beta/user-guide.md` around lines 36 - 38, The sentence repeats "when you want to"; update the phrasing to improve flow by changing the first occurrence to something like "If you prefer to skip the picker" and keep the rest but simplify to "Use --agent codex-cli, --agent claude, or --agent github-copilot instead of --editor to open an agent directly," referencing the flags --agent and --editor and the agent names codex-cli, claude, github-copilot so the meaning stays identical while avoiding repetition.
🤖 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 `@src/commands/workspace/open-view.ts`:
- Around line 288-307: interactiveCreate currently allows prompting even in JSON
mode; update its definition to also require that output is not JSON by adding a
JSON-mode guard (e.g. && !isJsonOutput(options) or &&
!resolveJsonOutput(options) or, if no helper exists, && !options.json). Change
the interactiveCreate assignment (the const interactiveCreate = ... line) so it
checks options for JSON mode in addition to resolveNoInteractive(options) and
isInteractive(options) so linksForNewWorkspace prompting is suppressed when
--json is set.
In `@src/core/workspace/open-surface.ts`:
- Around line 304-331: The cleanup currently removes any line equal to
getWorkspaceCodeWorkspaceFileName(workspaceName), which can delete user-authored
.gitignore entries; change the logic to only remove lines that were written by
our tool by matching a stable marker (e.g. "#{workspace-generated}") appended
when we create the ignore entry, and skip removing plain user lines. Concretely:
when writing the generated pattern (the code path that calls
FileSystemUtils.writeFile and uses getWorkspaceCodeWorkspaceFileName), append a
recognizable comment marker to that line; then update this removal block to only
filter out lines that match the generated pattern plus the marker (not the bare
filename), and if no marker is found leave the .gitignore untouched to preserve
user intent.
---
Nitpick comments:
In `@docs/workspaces-beta/user-guide.md`:
- Around line 36-38: The sentence repeats "when you want to"; update the
phrasing to improve flow by changing the first occurrence to something like "If
you prefer to skip the picker" and keep the rest but simplify to "Use --agent
codex-cli, --agent claude, or --agent github-copilot instead of --editor to open
an agent directly," referencing the flags --agent and --editor and the agent
names codex-cli, claude, github-copilot so the meaning stays identical while
avoiding repetition.
In `@src/commands/workspace/open.ts`:
- Around line 57-60: The isCodexCliOpener function casts opener.id to string
unnecessarily and checks a legacy 'codex' value; remove the cast by using
opener.id directly (typed as WorkspaceAgentOpenerId | WorkspaceEditorOpenerId)
and simplify the condition to only check for the normalized value 'codex-cli'
when opener.kind === 'agent' (i.e., in isCodexCliOpener use opener.id ===
'codex-cli' and drop the 'codex' branch); reference: function isCodexCliOpener
and type WorkspacePreferredOpener / WorkspaceAgentOpenerId.
🪄 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: 1b1b5f8c-f9dc-4197-bce8-bade1ce935e3
📒 Files selected for processing (55)
docs/cli.mddocs/concepts.mddocs/workspaces-beta/agent-cli-playbook.mddocs/workspaces-beta/user-guide.mdopenspec/initiatives/context-store-and-initiatives/roadmap.mdopenspec/initiatives/context-store-and-initiatives/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/11-manual-beta-reality-pass/notes.mdopenspec/initiatives/context-store-and-initiatives/work-items/11-manual-beta-reality-pass/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/11-manual-beta-reality-pass/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/12-context-store-first-run-and-cleanup-ux/evidence.mdopenspec/initiatives/context-store-and-initiatives/work-items/12-context-store-first-run-and-cleanup-ux/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/12-context-store-first-run-and-cleanup-ux/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/13-agent-handoff-output-and-delivery-polish/evidence.mdopenspec/initiatives/context-store-and-initiatives/work-items/13-agent-handoff-output-and-delivery-polish/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/13-agent-handoff-output-and-delivery-polish/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/14-workspaces-beta-guide-split/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/14-workspaces-beta-guide-split/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/evidence.mdopenspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/16-add-escalation-ux/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/16-add-escalation-ux/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/17-harden-team-shared-coordination/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/17-harden-team-shared-coordination/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/18-explore-initiative-hosted-target-bound-change-artifacts/evidence.mdopenspec/initiatives/context-store-and-initiatives/work-items/18-explore-initiative-hosted-target-bound-change-artifacts/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/18-explore-initiative-hosted-target-bound-change-artifacts/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/proposed-initiative-next-agent-handoff-ux/evidence.mdopenspec/initiatives/context-store-and-initiatives/work-items/proposed-initiative-next-agent-handoff-ux/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/proposed-initiative-next-agent-handoff-ux/tasks.mdsrc/commands/context-store.tssrc/commands/workspace.tssrc/commands/workspace/open-target-selection.tssrc/commands/workspace/open-view.tssrc/commands/workspace/open.tssrc/commands/workspace/opener-selection.tssrc/commands/workspace/operations.tssrc/commands/workspace/registration.tssrc/commands/workspace/selection.tssrc/commands/workspace/setup-prompts.tssrc/core/completions/command-registry.tssrc/core/context-store/foundation.tssrc/core/context-store/operations.tssrc/core/workspace/foundation.tssrc/core/workspace/open-surface.tssrc/core/workspace/openers.tstest/commands/context-store.test.tstest/commands/workspace-initiative-open.test.tstest/commands/workspace-open.test.tstest/commands/workspace.interactive.test.tstest/commands/workspace.test.tstest/core/context-store/foundation.test.tstest/core/workspace/foundation.test.tstest/core/workspace/legacy-state.test.tstest/helpers/path-env.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/core/workspace/open-surface.ts`:
- Around line 307-315: The current logic uses nonEmptyLines and
isPureLegacyGeneratedFile to only delete gitignore when it's the sole legacy
line; instead, detect if any line equals legacyGeneratedPattern and remove just
that line: use nonEmptyLines.includes(legacyGeneratedPattern) (or find) to
decide whether to act; if no match return; otherwise create a filteredLines =
nonEmptyLines.filter(l => l.trim() !== legacyGeneratedPattern), then if
filteredLines is empty call await fs.rm(gitignorePath, { force: true }) else
write the trimmed filteredLines back to gitignorePath (preserving other rules).
Ensure you update references to isPureLegacyGeneratedFile and only use fs.rm
when the resulting file would be empty.
🪄 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: b5af92af-6047-4dc1-a9bb-9fa3216d0ec3
📒 Files selected for processing (11)
docs/cli.mddocs/concepts.mdopenspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/tasks.mdopenspec/specs/workspace-foundation/spec.mdsrc/commands/workspace/open-view.tssrc/commands/workspace/setup-prompts.tssrc/core/workspace/foundation.tssrc/core/workspace/open-surface.tstest/commands/workspace.interactive.test.tstest/core/workspace/foundation.test.ts
💤 Files with no reviewable changes (2)
- openspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/plan.md
- openspec/initiatives/context-store-and-initiatives/work-items/15-context-store-project-roots-and-schema-led-initiatives/tasks.md
✅ Files skipped from review due to trivial changes (1)
- docs/concepts.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/commands/workspace/setup-prompts.ts
- src/commands/workspace/open-view.ts
- src/core/workspace/foundation.ts
- docs/cli.md
- test/core/workspace/foundation.test.ts
| const nonEmptyLines = existingLines.filter((line) => line.trim().length > 0); | ||
| const isPureLegacyGeneratedFile = | ||
| nonEmptyLines.length === 1 && nonEmptyLines[0]?.trim() === legacyGeneratedPattern; | ||
|
|
||
| if (!isPureLegacyGeneratedFile) { | ||
| return; | ||
| } | ||
|
|
||
| await fs.rm(gitignorePath, { force: true }); |
There was a problem hiding this comment.
Legacy ignore rule cleanup skips mixed .gitignore files.
At Line 311, cleanup returns unless the file is “pure legacy”, so the legacy <workspace-name>.code-workspace ignore line remains when users also have other ignore entries. This violates the “remove legacy rule when present, preserve unrelated rules” behavior.
Suggested fix
const legacyGeneratedPattern = getWorkspaceCodeWorkspaceFileName(workspaceName);
const existingContent = await fs.readFile(gitignorePath, 'utf-8');
const existingLines = existingContent.split(/\r?\n/u);
- const nonEmptyLines = existingLines.filter((line) => line.trim().length > 0);
- const isPureLegacyGeneratedFile =
- nonEmptyLines.length === 1 && nonEmptyLines[0]?.trim() === legacyGeneratedPattern;
-
- if (!isPureLegacyGeneratedFile) {
+ let removedLegacyPattern = false;
+ const keptLines = existingLines.filter((line) => {
+ if (line.trim() === legacyGeneratedPattern) {
+ removedLegacyPattern = true;
+ return false;
+ }
+ return true;
+ });
+ if (!removedLegacyPattern) {
return;
}
- await fs.rm(gitignorePath, { force: true });
+ while (keptLines.length > 0 && keptLines[keptLines.length - 1]?.trim().length === 0) {
+ keptLines.pop();
+ }
+ if (keptLines.every((line) => line.trim().length === 0)) {
+ await fs.rm(gitignorePath, { force: true });
+ return;
+ }
+ await FileSystemUtils.writeFile(gitignorePath, `${keptLines.join('\n')}\n`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nonEmptyLines = existingLines.filter((line) => line.trim().length > 0); | |
| const isPureLegacyGeneratedFile = | |
| nonEmptyLines.length === 1 && nonEmptyLines[0]?.trim() === legacyGeneratedPattern; | |
| if (!isPureLegacyGeneratedFile) { | |
| return; | |
| } | |
| await fs.rm(gitignorePath, { force: true }); | |
| const legacyGeneratedPattern = getWorkspaceCodeWorkspaceFileName(workspaceName); | |
| const existingContent = await fs.readFile(gitignorePath, 'utf-8'); | |
| const existingLines = existingContent.split(/\r?\n/u); | |
| let removedLegacyPattern = false; | |
| const keptLines = existingLines.filter((line) => { | |
| if (line.trim() === legacyGeneratedPattern) { | |
| removedLegacyPattern = true; | |
| return false; | |
| } | |
| return true; | |
| }); | |
| if (!removedLegacyPattern) { | |
| return; | |
| } | |
| while (keptLines.length > 0 && keptLines[keptLines.length - 1]?.trim().length === 0) { | |
| keptLines.pop(); | |
| } | |
| if (keptLines.every((line) => line.trim().length === 0)) { | |
| await fs.rm(gitignorePath, { force: true }); | |
| return; | |
| } | |
| await FileSystemUtils.writeFile(gitignorePath, `${keptLines.join('\n')}\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/core/workspace/open-surface.ts` around lines 307 - 315, The current logic
uses nonEmptyLines and isPureLegacyGeneratedFile to only delete gitignore when
it's the sole legacy line; instead, detect if any line equals
legacyGeneratedPattern and remove just that line: use
nonEmptyLines.includes(legacyGeneratedPattern) (or find) to decide whether to
act; if no match return; otherwise create a filteredLines =
nonEmptyLines.filter(l => l.trim() !== legacyGeneratedPattern), then if
filteredLines is empty call await fs.rm(gitignorePath, { force: true }) else
write the trimmed filteredLines back to gitignorePath (preserving other rules).
Ensure you update references to isPureLegacyGeneratedFile and only use fs.rm
when the resulting file would be empty.
alfred-openspec
left a comment
There was a problem hiding this comment.
Looks good to me. The context-store/workspace flow changes are covered by the updated tests, and CI is green.
|
Actionable comments posted: 0 |
alfred-openspec
left a comment
There was a problem hiding this comment.
Looks good. I checked the workspace/context-store flow changes, CI is green, and the targeted local test pass matches the PR validation scope.
* Polish beta context workspace flow * Allow context-only initiative workspace open * Add workspace beta compatibility review item
Summary
workspace openinitiative-aware, including lazy workspace creation, linked repo prompts, clearer opener behavior, and Codex CLI sandbox handling.Why
The manual beta pass exposed first-run friction around where context stores are created, how users discover initiatives from
workspace open, how Codex receives writable roots, and whether workspaces appear to own durable planning artifacts. This keeps the beta flow oriented around context stores/initiatives as shared context and workspaces as local views.Validation
pnpm run buildpnpm run lintpnpm exec vitest run test/commands/context-store.test.ts test/commands/workspace.interactive.test.ts test/commands/workspace-initiative-open.test.ts test/commands/workspace-open.test.ts test/commands/workspace.test.ts test/core/context-store/foundation.test.ts test/core/workspace/foundation.test.ts test/core/workspace/legacy-state.test.ts test/core/planning-home.test.ts test/commands/artifact-workflow.test.tsSummary by CodeRabbit
Documentation
New Features
Chores / Tests