Repository navigation
feat(cli): add repo rm, repo set, and repo group management verbs - #8790
briancripe wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughAdds CLI support for repository grouping and project-group management. The change defines command specifications, parses group-related flags, resolves groups by ID or name, formats group results, and registers new handlers. It adds Merge Risk: ⚪ Minimal · up to The CLI adds repository and project-group management through existing runtime operations, with no actionable merge-blocking risk remaining after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 59d906fb-d040-4493-8ddd-268d8ac0652f
📒 Files selected for processing (16)
skill-guides/orca-cli.mdskills/orca-cli/SKILL.mdsrc/cli/args.tssrc/cli/bundled-skill-guides.tssrc/cli/dispatch.tssrc/cli/format.tssrc/cli/handlers/repo-group.test.tssrc/cli/handlers/repo-group.tssrc/cli/handlers/repo.tssrc/cli/help.tssrc/cli/project-group-selector.test.tssrc/cli/project-group-selector.tssrc/cli/specs/core.tssrc/cli/specs/repo.tssrc/cli/workspace-format.test.tssrc/cli/workspace-format.ts
5378808 to
c1cd913
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
cc241f0 to
ac5f80d
Compare
Readiness verdict: code-ready; GitHub status remains a merge blockerRebased and force-pushed with exact leases onto current Findings
Reviewed crash/cast/retry/unbounded-growth, security/supply-chain, performance/resources, functional correctness, backward compatibility/data loss/mobile impact, folder workspaces, SSH ownership, mixed-version remote wire behavior, GitHub/GitLab neutrality, and macOS/Linux/Windows. No remaining proven P0/P1/P2 finding. Tests by machinemacOS local
Windows
Visual proofN/A — CLI/runtime-only change; no rendered UI changed. Residual risk / exact merge blockerGitHub still reports |
ac5f80d to
04c396c
Compare
The CLI could add repos but not remove them, and project groups had no CLI surface at all despite full projectGroup.* RPC support in the runtime. Adds thin wrappers over the existing RPCs so orca-data.json stays owned by the app's Store: - repo rm (aliases remove/delete): unregister a repo from Orca; files and checkouts stay on disk - repo set: --group/--ungroup membership via repo.update, plus --display-name and --badge-color (validated client-side because the store silently drops unnormalizable colors) - repo group list/create/set/rm: group lifecycle including --parent-group nesting, rename, --color null clearing, and subtree delete Group selectors (id:/name:/bare) resolve client-side before any mutating call because Store.updateRepo and moveProjectToGroup silently null unknown group ids - a typo'd selector must fail loudly instead of silently ungrouping a repo. Repo specs move from specs/core.ts to a new specs/repo.ts to stay under the max-lines limit.
The handler printed the success message regardless of the removed flag. The current runtime throws repo_not_found instead of returning false, but handle the degenerate response the same way repo group rm handles deleted: false so an older or future runtime cannot read as success.
Without expect.unreachable the catch body never runs on a regression that resolves a duplicated bare name, letting the test pass vacuously.
04c396c to
e6dae3a
Compare
Readiness verdict — PR #8790Verdict: code-ready; CI pending. Final head |
Readiness verdict: ready to merge on content — head
|
| Gate | Result |
|---|---|
oxfmt --check on help.ts, specs/core.ts, specs/repo.ts |
pass |
oxlint src/cli src/main/runtime --deny-warnings |
pass |
pnpm run typecheck:cli |
pass |
vitest run src/cli |
93 files / 937 tests passed |
vitest run src/main/runtime/runtime-repo-group-host-scope.test.ts src/main/runtime/rpc/methods |
96 files / 959 tests passed |
pnpm run verify:bundled-skill-guides |
pass |
pnpm run build:cli |
pass (verify-cli-bin OK) |
Real built-CLI smoke on the rebased head confirms both sides survived the merge:
node out/cli/index.js repo group --helplistsset,rm,group list,group create,group set,group rm.node out/cli/index.js worktree rm --helprenders main's new--force … does not force branch deletiontext and both branch-cleanup notes.
Windows awin evidence
Carried forward from the prior review of 04c396c0da9, and still valid: the rebased tree differs from that Windows-validated head only by main's own two documentation hunks in help.ts / specs/core.ts. No CLI repo/group code path, runtime host-scope check, RPC field, or handler changed in the rebase. The previously recorded Windows run on awin (Orca 1.4.186-hourly.202608200132) — build:cli, typecheck:cli, 4 critical suites / 59 tests, plus the end-to-end node out/cli/index.js workflow covering repo registration, root/nested group create, assignment, display-name and badge-color normalization (369 → #336699), bogus-selector exit 1 with unchanged state, ungroup, group deletion, and repo deregistration with .git preserved — therefore still describes this head's behavior.
Findings
- P0: none.
- P1: none unresolved. Both previously reported P1s (cross-host
repo.updaterow targeting; direct-SSH mutations reaching sibling/local records) remain fixed and their regression tests pass on this head. - P2: none unresolved.
repo group set --colorhex normalization remains in place and covered.
No new finding was introduced by the rebase; the resolution is documentation text only.
Remaining blocker
Not a code blocker — CI has not queued yet. statusCheckRollup is empty ~5 minutes after the force-push, which is why mergeStateStatus reads UNSTABLE rather than CLEAN. This is a fork-PR workflow-authorization/queue delay, not a failure. Once required checks run and pass, the PR should reach CLEAN with no further changes needed.
Visual proof
N/A — CLI/runtime-only change; no rendered UI changed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/runtime/orca-runtime.ts (1)
20749-20788: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the connection-scoped project-group lookup into one helper.
updateProjectGroupanddeleteProjectGroupboth re-implement the same lookup: find the group byid, comparecandidate.connectionId?.trim() || nullagainst the caller-suppliedconnectionId, and bail out if it does not match.createProjectGroupnormalizes a parent group'sconnectionIdthe same way a few lines above.Keep this normalization in one place. A future edit to one of the three call sites (for example, a change to how empty strings are treated) can silently diverge from the other two and reopen the host-mismatch class of bug this PR fixes.
Extract a private helper, for example
findProjectGroupForConnection(groupId, connectionId), and call it fromupdateProjectGroup,deleteProjectGroup, andcreateProjectGroup's parent lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7450d063-506c-4f71-8780-0b5f549038f6
📥 Commits
Reviewing files that changed from the base of the PR and between c1cd91373534d746a7407f5acc356b9e1457976d and e6dae3a.
📒 Files selected for processing (16)
src/cli/dispatch.tssrc/cli/handlers/core.test.tssrc/cli/handlers/repo-group.test.tssrc/cli/handlers/repo-group.tssrc/cli/handlers/repo.tssrc/cli/help.tssrc/cli/index.tssrc/cli/project-group-selector.test.tssrc/cli/project-group-selector.tssrc/cli/skills.test.tssrc/cli/specs/core.tssrc/cli/workspace-format.test.tssrc/cli/workspace-format.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/repo.tssrc/main/runtime/runtime-repo-group-host-scope.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/help.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.
|
+1 on getting this merged. Concrete case: I have 118 repos registered in Orca 1.4.215 on one Mac, and the only structure the sidebar offers is Project Groups, which today exist only as manual UI actions (right-click, "New group from project" / "Move to group"). What I actually want is to let an agent do the organizing, not do it by hand. I already drive Orca from Claude Code and Codex through Since the last readiness verdict on this PR was "ready to merge on content, CI pending" a month ago, is there anything blocking it beyond CI? Happy to rebase-test the branch locally against current |
|
Supporting this from an agent-orchestration workflow. We can register repos and launch workers from the CLI, but cannot finish organising those repos into their existing project groups. That leaves a manual desktop step in an otherwise headless workflow; putting the preference in an agent skill does not help when the command is missing. The group listing and assignment commands here would cover that gap. Resolving group selectors before mutation, especially rejecting an unknown group without silently ungrouping the repo, is particularly useful for agents. Related use case: #8766. I have checked the missing surface in the installed CLI, but have not tested this PR's current head. |
Summary
The
orcaCLI could register repos (repo add) but never remove them, and project groups — a first-class app feature with fullprojectGroup.*RPC support — had no CLI surface at all. This adds the missing management verbs as thin wrappers over the existing runtime RPCs, soorca-data.jsonstays owned by the app'sStore(no new persistence, schema, or backend code):orca repo rm --repo <selector>(aliasesremove/delete,destructive): unregisters the repo from Orca. Files and git checkouts stay on disk; spec notes call out that Orca worktree metadata is dropped irreversibly.orca repo set --repo <selector> [--group <selector>|--ungroup] [--display-name <name>] [--badge-color <hex>]: repo settings via the existingrepo.updateRPC. Group membership is modeled as a repo attribute, matching the UI ("Move to group"/"Remove from group" live on the project context menu).orca repo group list|create|set|rm: project-group lifecycle —--parent-groupnesting, rename,--color nullclearing, and subtree delete (repos in deleted groups are kept and become ungrouped, matching the UI behavior).Group selectors accept
id:<groupId>,name:<name>, or a bare id/unique name, mirroring the existing repo-selector semantics. Resolution happens client-side before any mutating RPC becauseStore.updateRepoandStore.moveProjectToGroupsilently normalize an unknownprojectGroupIdtonull— without this guard a typo'd selector would silently ungroup a repo.--badge-coloris likewise validated client-side (sharednormalizeRepoBadgeColor) because the store silently drops unnormalizable colors, which would otherwise report a no-op success.specs/core.tsrepo specs moved to a newspecs/repo.tsto stay under the max-lines limit (no suppressions added). Bundled skill guides (src/cli/bundled-skill-guides.ts,skills/orca-cli/SKILL.md) regenerated after updatingskill-guides/orca-cli.md.Screenshots
No visual change (CLI-only). Sample output:
Testing
pnpm lintpnpm typecheckpnpm test— 29,924 passed; the only 5 failures (config/scripts/publish-complete-draft-releases.test.mjs,config/scripts/release-rc-history.test.mjs,src/shared/posix-command-path-lookup.test.ts) fail identically on the base commit without this change (local git-tag state / local zsh PATH), confirmed by running them on the unmodified basepnpm buildproject-group-selector.test.ts(id/name/bare/ambiguous/not-found resolution),handlers/repo-group.test.ts(asserts the mutating RPC is not issued when selector resolution fails — the silent-ungroup regression guard — plus flag-validation and vanished-group error paths),workspace-format.test.ts(formatters). Registry-parity and vocabulary-policy suites enforce the new specs/handlers/aliases automatically.Verified end-to-end against a live dev app (
orca-dev): add repo → group create →repo set --group→ rename/recolor → nested--parent-groupchild →--ungroup→ subtreegroup rm→repo rm(via therepo deletealias), confirming the scratch repo survived on disk and a bogus--groupselector exits 1 without mutating anything.AI Review Report
Reviewed with Claude Code. Checks and outcomes:
src/cli; no keyboard shortcuts, labels, shell invocations, or filesystem paths were added. The only path-handling call sites (repo add) are untouched. The CLI transport (unix socket / named pipe) is unchanged.--environment) runtimes; group-selector resolution uses the sameprojectGroup.listRPC channel. No local-only assumptions introduced.rmcanonical withremove/deletealiases,show/setconventions);destructive: trueis set so typo-recovery never steers agents into removal; agent-context schema output is generated from the specs automatically. No git-provider-specific behavior.projectGroup.listRPC before a mutating call; group lists are small and these are interactive one-shot commands. No hot paths touched.Store.updateRepo/moveProjectToGroupsilently null unknown group ids → client-side resolution that fails loudly, with tests asserting no mutating RPC is issued on failure; (2)projectGroup.update/deletereturn{group: null}/{deleted: false}instead of errors for unknown ids → handlers convert these to explicit failures; (3) invalid badge colors are silently dropped by the store → client-side validation.Security Audit
normalizeRepoBadgeColor, at-least-one-update-flag and mutually-exclusive-flag checks); server-side Zod schemas on the existing RPCs are unchanged and remain the trust boundary.repo rmdeletes no files by design (unregister only), which the spec notes state explicitly.repo rmandrepo group rmare markeddestructive(excluded from typo-suggestion recovery) and their specs document exactly what is and is not removed.Notes
// Why:comments in the repo's idiom.X handle: @BrianCripe
ELI5
CLI can unregister repos and manage project groups (
repo rm,repo set, group verbs). Headless workflows match more of what the desktop app already could do.