feat(ui): worktrees page owners and creation, structured snapshot failures, preserved-checkout cleanup#103526
Conversation
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dafcddfb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.createOpen = !this.createOpen; | ||
| if (this.createOpen && !this.createRepoRoot) { | ||
| const agents = this.context.agents.state.agentsList; | ||
| const defaultAgent = agents?.agents.find((agent) => agent.id === agents.defaultId); |
There was a problem hiding this comment.
When the operator has selected a non-default agent, this still seeds the create form from agents.defaultId, so clicking New worktree can create a checkout for the default agent's repository instead of the active agent's workspace. The app already tracks the selected agent via context.agentSelection.state.selectedId; use that before falling back to the configured default so the new form matches the current agent context.
Useful? React with 👍 / 👎.
bec30b1 to
04016c5
Compare
9dafcdd to
d8a225a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8a225aacb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .request<WorktreeBranchesResult>("worktrees.branches", { repoRoot }) | ||
| .then((result) => { | ||
| if (this.isOperationScopeCurrent(scope) && repoRoot === this.createRepoRoot.trim()) { | ||
| this.createBranches = result.branches.map((branch) => branch.name); |
There was a problem hiding this comment.
When the branch list contains a remote-only entry, this stores only branch.name and createWorktree() later sends that bare name as baseRef. git worktree add -h documents --guess-remote as trying to match the new branch name with a remote-tracking branch, and with only origin/feature-a present git worktree add -b openclaw/foo ... feature-a creates/checks out local feature-a instead of the openclaw/foo branch the service records. That leaves the registry pointing at a branch that was never created, so later removal tries to delete openclaw/foo and fails after the checkout is removed; preserve kind and pass a full remote ref such as origin/<name> for remote picker entries.
Useful? React with 👍 / 👎.
04016c5 to
0ee43cb
Compare
d8a225a to
6120545
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6120545bc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!deleted || !session.active) { | ||
| // Dirty/unpushed checkouts survive the delete; offer an explicit force | ||
| // removal instead of silently orphaning them under the state dir. | ||
| if (outcome.worktreePreserved) { |
There was a problem hiding this comment.
This handles preserved worktrees only for the sidebar delete path. The Sessions page uses context.sessions.deleteMany() for both selected deletes and its row menu (ui/src/pages/sessions/sessions-page.ts:550 and ui/src/pages/sessions/sessions-page.ts:612-618), and deleteMany() still drops the worktreePreserved value from sessions.delete; deleting a dirty worktree-backed session there leaves the checkout preserved with no force-delete prompt, so the orphaned-checkout flow remains outside the sidebar.
Useful? React with 👍 / 👎.
6120545 to
a4201b0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4201b067e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.isOperationScopeCurrent(scope)) { | ||
| this.creating = false; | ||
| await this.load(); |
There was a problem hiding this comment.
When a create request is in flight and the Gateway disconnects/reconnects or the page is rebound, invalidateOperations() bumps operationEpoch, so this guarded finally block is skipped and creating stays true. The New worktree button then remains disabled for the rest of the page lifetime even though the replacement connection is usable; reset creating when invalidating operations or outside the stale-scope guard.
Useful? React with 👍 / 👎.
ee14841 to
d79a2a6
Compare
a4201b0 to
2b3d058
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b3d0581f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!deleted || !session.active) { | ||
| // Dirty/unpushed checkouts survive the delete; offer an explicit force | ||
| // removal instead of silently orphaning them under the state dir. | ||
| if (outcome.worktreePreserved) { |
There was a problem hiding this comment.
When the Gateway reconnects or the context is rebound after sessions.delete returns but before the capability's refresh() finishes, context.sessions.delete() can return deleted: false while still carrying worktreePreserved; this branch runs before the !outcome.deleted guard below, so the sidebar can prompt and send a force worktrees.remove through the replacement client for a stale delete epoch. Check outcome.deleted before offering the force removal, or drop preserved worktree data when the delete result is stale.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: found issues before merge. Reviewed July 10, 2026, 7:18 AM ET / 11:18 UTC. Summary PR surface: Source +545, Tests +44, Docs 0. Total +589 across 73 files. Reproducibility: yes. for the review defects: the current-head source deterministically shows the selected-agent mismatch and both reconnect/freshness failures without needing environment-specific data. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Preserve the owner-aware Worktrees UI and structured snapshot contract, but bind creation and deletion follow-up to the selected agent and originating Gateway epoch, with focused reconnect regressions before merge. Do we have a high-confidence way to reproduce the issue? Yes for the review defects: the current-head source deterministically shows the selected-agent mismatch and both reconnect/freshness failures without needing environment-specific data. Is this the best way to solve the issue? No, not on the current head; typed snapshot outcomes and owner-aware management are the right design, but the implementation must preserve selected-agent and Gateway-epoch invariants. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7b03a2ae201a. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +545, Tests +44, Docs 0. Total +589 across 73 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
487f718 to
9a35da8
Compare
06c824d to
11ea65a
Compare
…lures, preserved-checkout cleanup
- Worktrees settings page gains an Owner column (manual/Workboard/session
with a link into the owning chat) and a New worktree form with repo
default, optional name, and a worktrees.branches-backed base picker
- non-forced worktrees.remove snapshot failures are a structured
{ removed: false, snapshotError } result (typed WorktreeSnapshotError in
the service) instead of error-string sniffing in the UI
- deleting a session whose dirty/unpushed checkout was preserved now
reports worktreePreserved through the session capability and offers an
explicit force removal instead of silently orphaning the checkout
Part of #103431
deleteMany now aggregates worktreePreserved results, and the Sessions page surfaces one notice pointing at Settings -> Worktrees instead of silently orphaning dirty checkouts during bulk deletion. Part of #103431
2b3d058 to
fecf6f4
Compare
|
Merged via squash.
|
|
Before/after screenshots for the Worktrees page (deterministic mock-gateway harness, identical seeded records): Before — no Owner column, no create form (the stacked layout is a pre-existing breakpoint bug, fixed in #103899):
After — Owner column (Session link / Workboard / Manual) and the New worktree form with base-branch picker:
|
…lures, preserved-checkout cleanup (openclaw#103526) * feat(ui): worktrees page owners and creation, structured snapshot failures, preserved-checkout cleanup - Worktrees settings page gains an Owner column (manual/Workboard/session with a link into the owning chat) and a New worktree form with repo default, optional name, and a worktrees.branches-backed base picker - non-forced worktrees.remove snapshot failures are a structured { removed: false, snapshotError } result (typed WorktreeSnapshotError in the service) instead of error-string sniffing in the UI - deleting a session whose dirty/unpushed checkout was preserved now reports worktreePreserved through the session capability and offers an explicit force removal instead of silently orphaning the checkout Part of openclaw#103431 * fix(ui): report preserved checkouts from batch session deletes deleteMany now aggregates worktreePreserved results, and the Sessions page surfaces one notice pointing at Settings -> Worktrees instead of silently orphaning dirty checkouts during bulk deletion. Part of openclaw#103431 * fix(ui): reconcile generated locale artifacts after rebase
1 participant
Related: #103431
Stacked on #103498 (sidebar redesign) and #103432 (protocol/server foundation).
What Problem This Solves
The Worktrees settings page listed checkouts without saying who owns them, offered no way to create one, and decided whether to offer a force delete by string-matching "snapshot failed" inside error messages. Deleting a session whose worktree had uncommitted or unpushed work silently left the checkout orphaned under the state directory with no UI pointing back at it.
Why This Change Was Made
The Worktrees page gains an Owner column (Manual, Workboard, or Session with a link into the owning chat) and a New worktree form that defaults to the selected agent's workspace and offers a
worktrees.branches-backed base-ref picker. Non-forced removals that fail their safety snapshot now return a structured{ removed: false, snapshotError }result — the service throws a typedWorktreeSnapshotErrorthat the gateway handler maps — so the force-retry decision stops depending on error-message wording. Session deletion surfaces the foundation PR'sworktreePreservedresult: the sidebar explains that the dirty checkout was kept (branch included) and offers an explicit force removal.User Impact
Operators can see which worktree belongs to which session and jump to it, create worktrees from the UI on any base branch, and no longer leak forgotten checkouts when deleting sessions — keeping dirty work stays the default, deleting it is one explicit confirm.
Evidence
?session=agent:main:dashboard:…; New worktree defaulted to the agent workspace, listed branchesmain/feature-base/openclaw/live-test/topic-x, and createdmanual-oneon basetopic-x(Owner "Manual"). After dirtying the session worktree and deleting the session, the UI confirmed "The session's worktree has uncommitted or unpushed work, so it was kept (openclaw/live-test). Delete the checkout anyway?" and the forced removal deleted both checkout and branch.pnpm test ui/src/pages/worktrees/worktrees-page.test.ts src/gateway/server-methods/worktrees.test.ts src/agents/worktrees/service.test.ts ui/src/components/app-sidebar.test.ts— green, including new coverage for the structuredremoved=falsesnapshot outcome and the force-retry flow.pnpm ui:buildclean; locale bundles regenerated with the keyless raw-copy baseline.