Skip to content

fix(repos): deleting a project with live terminals warns and needs --force - #27172

Merged
OrcaWin merged 4 commits into
mainfrom
OrcaWin/d4-d4-i4b
Oct 10, 2026
Merged

OrcaWin merged 4 commits into
mainfrom
OrcaWin/d4-d4-i4b

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

ELI5

Removing a project from the command line used to happen silently even if terminals were still running in it. Now the command stops and tells you what you would lose, and you add --force if you really mean it. The remove dialog in the app also tells you how many open terminals will be closed.

What Changed

Before: orca project setup-delete --setup <id> on a project backed by a registered repo dropped the repo at once. Any terminals still running in its workspaces kept running with no project behind them, and the saved workspace details (names, comments, linked issues, ordering) were deleted. The sidebar "Remove Project" dialog said only that files stay on disk.

After:

  • The CLI refuses when the project has terminals that are not known to have exited, or workspaces with saved details, and prints the counts plus "Re-run with --force to remove it anyway." With --force, or when nothing is in use, it removes as before.
  • The sidebar dialog adds a line such as "2 open terminals in this project will be closed." (the app already closes them on remove; now it says so).

Mechanism: projectHostSetup.delete takes a new optional force field. The CLI always sends it (false unless --force). When it is exactly false, the host that owns the project counts, before deleting:

  • its terminals for that repo on that host, using the existing worktree/host fence; a terminal whose host is unreachable counts as possibly live, never as exited;
  • workspace metadata for that repo on that host, read through the host-qualified accessor (so a same-id, same-path sibling host's rows count too) and filtered with the same host rule the delete uses to prune it.

A non-zero count throws a readable error instead of deleting. The counting lives in src/main/runtime/project-removal-impact.ts.

Why

  • The host decides, not the client. The terminals and metadata live on the execution host, so that host counts them at the moment of the delete. A client-side preflight would need several calls and could go stale before the delete.
  • Absent force keeps the old unchecked delete. Clients and hosts update independently. An older desktop app sends projectHostSetup.delete from its own confirmed UI and must not start getting refusals. An older host drops the unknown field, so a new CLI there behaves as it did before. The desktop UI keeps its own confirmation and now shows the terminal count.
  • Adapted from the design: the design says "setup-delete / repo remove". There is no repo remove CLI command; repo.rm is only called by the app's remove flow, which already confirms and closes terminals, so it is left alone. The "refresh identity in place" step listed as a prerequisite does not touch this code, so this PR does not depend on it.

Linked Issue

None. This is follow-up D4-3b in the owner-routing design.

Visual Proof

N/A for the CLI (text output). The dialog gains one line of text, built from existing tokens, under the description. Not captured: this machine is load-capped and cannot run Electron.

Testing

  • src/main/runtime/project-host-setup-delete-guard.test.ts: acceptance test. It fails on origin/main (2 of 6: the delete runs while terminals or metadata exist) and passes with this change. It also covers the --force path, the unused-project path, the older-client path (no force), and host and repo scoping for SSH and local.

  • src/cli/index-project-setup.test.ts: the CLI sends force: false / force: true.

  • src/renderer/src/store/repos/repo-removal.test.ts and RemoveFolderDialog.test.tsx: the dialog's terminal count is host-scoped and rendered.

  • pnpm tc, oxlint, check:code-quality:changed, localization catalog checks, verify:rpc-params-catalog.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

P0/P1 review: an independent reviewer found one P1. The guard read only the raw-id metadata map, so a sibling host with the same repo id and path looked empty and its details could be deleted without --force. Fixed by reading host-qualified metadata, with a regression test; the follow-up review of that fix found no P0/P1.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH: terminal counts are fenced to the project's own host, and lost contact counts as possibly live (not exited).
  • Folder workspaces: their repoId::path::workspace:<id> ids match by repo prefix.
  • Windows and WSL: no path handling changes.
  • Mixed versions: covered under Why.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@OrcaWin
OrcaWin requested a review from brennanb2025 as a code owner October 10, 2026 07:00
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 27b17481-d429-43a2-a96f-1058bc03beac


📥 Commits

Reviewing files that changed from the base of the PR and between a3adffda472ff024ee300894410c94e0367baa26 and ba5b7cb.



📒 Files selected for processing (1)
  • src/renderer/src/components/sidebar/RemoveFolderDialog.test.tsx


Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

Project setup deletion now accepts a --force flag. Without force, the runtime checks for live terminals and saved workspace metadata before deleting a setup. The folder removal dialog counts open terminal tabs for the selected repository and host, then displays a singular or plural warning when tabs are present.



Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to ba5b7

Current CLI users must use --force to bypass the terminal and workspace safeguard; older clients retain their established behavior. The inspected deletion and dialog paths use the selected repository and host, and no actionable merge risk remains.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 15 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: project deletion now warns about live terminals and requires --force.
Description check Passed The description is detailed and aligned with the template. It explains the user impact, implementation, rationale, testing, visual proof, review status, compatibility considerations, and checklist. Th…

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR



  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

m4air and others added 3 commits October 10, 2026 00:24
…force

orca project setup-delete now refuses a repo-backed project that still has
terminals not known to have exited or saved workspace details, unless --force.
The sidebar remove dialog says how many open terminals will close.

Co-Authored-By: Claude <noreply@anthropic.com>
…in the delete guard

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@OrcaWin
OrcaWin merged commit cb69d52 into main Oct 10, 2026
37 checks passed
qiragu added a commit to guwenqing/orca-bot-kit that referenced this pull request Oct 10, 2026
…tup-delete (#528) (#535)

Orca's next release (stablyai/orca#27172) refuses a plain project setup-delete of a project with saved workspace details or live terminals. A bot's project keeps saved details after its tabs close. obk retire --bot now sends --force, after its own checks: the project is the bot's own, and the kit closed the tabs the book names. Orca 1.4.223 refuses --force as an unknown flag and changes nothing; then the kit sends the plain delete. Every other refusal is reported as before.

Tier: 0 — obk retire keeps removing a bot's Orca project on the Orca release that guards setup-delete

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
qiragu added a commit to guwenqing/orca-bot-kit that referenced this pull request Oct 10, 2026
…the runner names any project a run leaves (#536) (#546)

* System tests remove their own Orca projects on the guarded Orca, and the runner names any project a run leaves (#536)

The system tests' teardowns sent a plain `project setup-delete` and ignored
the answer. Orca's next release refuses that for a project with saved
workspace details (stablyai/orca#27172), so every run would leave its
projects behind.

- test/helpers/own-project.js: deleteOwnProject(setup, bots) checks that
  the project is inside the test's own <tmp>/obk-system-* folder, then
  removes it with deleteProject from src/orca.js (#528: --force, and the
  plain delete on Orca 1.4.223).
- All 37 teardowns in test/system/ use it. A failed delete is kept, the
  rest of the teardown runs, and the teardown fails on it last.
- scripts/test-system.js lists Orca's projects after the run. It names
  each project in a folder the run made, with its setup id, and fails the
  run. It removes none. The architect replaced boundary 3 with this.
- test/system-teardowns.test.js holds the system tests to this, and now
  skips regular expression literals when it takes comments out.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The teardown check takes a division after a postfix ++ or --, or after a property named like a keyword, for a division (#536)

Found in the review of PR #546: `n++ / 2` and `obj.in / 2` were read as the
start of a regular expression, so a setup-delete later on the line was
hidden from the check.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The runner names only projects that were not in Orca before the run, and the teardown check sees a dot across whitespace (#536)

From the review of PR #546:
- An earlier run that left its project but removed its folder made the
  next run fail on that project. The runner now lists Orca's projects
  before the run too. A project counts only when its setup id is new and
  it is in a folder the run made. When the listing before the run fails,
  the runner says that it cannot tell, and names none.
- `object . in / 2`, with space or a comment around the dot, was still
  read as a regular expression. The keyword rule now looks across
  whitespace for a dot.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The teardown check never hides code behind a misread slash (#536)

From the review of PR #546: `{} / 2`, `of / 2`, `πin / 2` and a later
regex on the same line still hid a delete, because text after an
ambiguous slash was erased. Now a slash keeps its text as code, and only
its quotes are made neutral. The rest of that line counts as code, and a
string ends at the end of its line. Forbidden calls are also looked for in
a second reading with no regex handling.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* A system test may not name setup-delete or deleteProject( at all, read on the raw text (#536)

From the review of PR #546: a backtick in a regular expression hid a
delete from both readings of the parser. No text heuristic for
JavaScript is complete, so the #536 rule no longer uses one. It reads
the raw source, comments and strings included. No file in test/system/
names either text today. The teardown-scoped rules still use the parser.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The teardown check escapes names fully, and credits the raw-text rule rightly (#536)

From the approval of PR #546: two comments said the owner ruled on the
raw-text rule; it was the developer's decision, accepted in review.
CodeQL flagged two regexes built from names with only `$` escaped. They
now escape every metacharacter.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant