Repository navigation
feat(web-shell): unblock git update on dirty working tree - #10390
Conversation
The workspace "Update Project" action ran a plain git pull, so any uncommitted changes left users with a raw dirty_working_tree error and no way forward outside a terminal. The pull endpoint accepts two opt-in resolutions, the two things a user would do in a terminal: stash the local changes (including untracked files) around the pull and restore them by identity, or discard them and fast-forward. Both are refused while a merge, rebase, cherry-pick, revert or am is parked in the worktree, the discard is validated (fetch + ancestor check) before anything is destroyed, and a failed stash pull aborts only the merge or rebase it started before restoring the entry. The branch picker offers the two resolutions inline when the plain pull is blocked, with the destructive one behind a confirming click, and renders the daemon's message for every other refusal. Ambient git configuration, ignored-file semantics and concurrent pulls are deliberately left to git; docs/design records each as a non-goal.
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
…pull
Address the round-1 review of the resolution flows, by identity rather
than by adding guards:
- The auto-stash is captured by provenance (the new entry carrying the
auto-stash message, from a before/after listing), never as "the top of
refs/stash", so a terminal push landing in the gap is left alone.
- The drop checks the SHA git reports as dropped; if the slots shifted
under it, the other entry is stored back and ours is reported as kept.
- Failure recovery aborts a merge or rebase only when its MERGE_HEAD or
rebase-*/onto points at the upstream tip the pull was integrating; the
sequencer probe runs immediately before stash push and reset --hard.
- The stash flow always throws a typed pull_failed after aborting, also
when nothing was stashed or git refused the stash itself, so the client
shows git's reason instead of re-offering the same resolution.
- The force flow fetches with --prune, re-verifies the upstream, and
integrates the validated tip with merge --ff-only @{upstream} instead
of fetching again after the discard.
- Successful responses are path-redacted like the failures; the conflict
result carries the stash SHA, which the popover now names.
- The popover keeps the panel on force_unsupported, keeps the restore
warning across a reopen, only dismisses the panel for a branch creation
that actually runs, and allows 420s for the multi-command flows.
Tests drive the concurrent interleavings deterministically through a
PATH git shim that injects a terminal's actions around one invocation.
|
Round-1 review addressed in
The concurrent interleavings are now pinned deterministically: a PATH Local runs on the pushed head: core
Two things outside this PR, for the record: the earlier web-shell visuals failure on |
|
🤖 Could not produce a passing fix for this feedback (round 1/100) — the verification gate rejected the attempt. This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. Autofix review round 2 — PR #10390Same-run verification repair round. The previous commit Same-run verification rejection: environmental, evidencedThe deterministic gate rejected
Why it was not pushed: tests failed in packages/cli 中文说明🤖 未能为该反馈产生可通过验证的修复(第 1/100 轮) —— 验证门拒绝了该尝试。此项现在需要人工处理;循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 验证门的拒绝原因与日志证据见上方英文部分(gate-rejection 不翻译)。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33208001835 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
… best-effort
Address the round-2 review of the resolution flows:
- Failure recovery aborts a merge or rebase only when git itself created
it: a pre-existing state makes `git pull` exit 128 before touching the
tree, so only an exit of 1 with the state pointing at the integrated
upstream tip identifies the pull's own. A same-tip merge a terminal
parked meanwhile is left in place with its staged resolution.
- Every recovery step is best-effort: a failed probe or listing never
turns a recovered repository into an unclassified error, the post-push
re-listing failure points at the entry by its message, and a failed
store-back after a drop shift names the displaced entry and the command
that recovers it.
- Both flows fetch (`--prune`) before checking the upstream, so a pruned
tracking ref heals when the remote branch exists again; a configured
upstream whose branch is gone is a typed refusal, while a branch with
no upstream keeps git's own message.
- The force flow fast-forwards to the validated SHA rather than the
symbolic `@{upstream}`, which a concurrent fetch can move.
- Kept-entry notices always carry the SHA; the restored arm of a failed
update carries the drop diagnostic; silent git failures name the usual
lock-file cause; the popover budget covers the 16-command worst case.
|
Round-2 review addressed in
Local runs on the pushed head: core |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间(timeout (3600000ms))(第 2/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33264363801 🧠 Handled by Qwen Code · model/模型 |
main moved the daemon React bindings out of @qwen-code/webui and into web-shell's own client/daemon-react-sdk, rewriting every import. Take main's path for useWorkspace and keep this branch's DaemonHttpError import; the pull API this branch extends still lives in @qwen-code/sdk/daemon, which the forked bindings re-use unchanged.
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 3/100) — it will retry on the next scan. Round-3 summaryThis round was scoped down per the budget warning (the previous round exhausted Implemented this round (commit 42feea9756)
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误(第 3/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33271344698 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 4/100) — it will retry on the next scan. Round summaryAddressed the round-4 review of the dirty-worktree pull flows (PR #10390). Commit Feedback points and dispositions
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误(第 4/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33280763433 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped after 5 consecutive rounds that pushed nothing (failed rounds, timeouts, gate rejections, or stops under instruction). Retrying at the same per-round budget is not converging — this usually means the PR is too large or conflicts with a fast-moving Round summary — PR #10390 (commit
|
| Finding | Disposition | What changed |
|---|---|---|
| R4-1 [Critical] killed pull leaves its own merge parked (rc:3887937825) | Fixed | A pull that dies without an exit code is now treated like a conflicted exit 1 when deciding whether to abort parked merge/rebase state, via a new pullExitCode helper in the stash flow's recovery; the tip-identity guard in abortOwnPullState is unchanged. One deliberate widening beyond the suggested .killed check: a signal-shaped death also counts. Node reports killed: true only for kills it initiated itself; an external or OOM kill arrives as signal with killed: false (verified by probing execFile's error shapes), and it leaves the identical stranded MERGE_HEAD with the same downstream history-corruption path. Witness: stash pull aborts the merge it started when the pull is killed mid-integration — the shim runs the real fetch, makes the conflicting merge that writes MERGE_HEAD, then dies by SIGTERM (the exact timeout-kill shape). Removing the kill handling leaves MERGE_HEAD parked and turns the test red. |
| R4-2 **[Critica |
中文说明
🤖 AutoFix 已停止:连续 5 轮未能推送任何内容(失败轮次、超时、验证门拒绝或按指示停止)。以相同的单轮预算重试并不收敛 —— 这通常意味着 PR 过大,或与快速变动的 main 冲突。应由人工 rebase、拆分或缩减它,然后评论 @qwen-code /retry 重新武装。在此之前,后续扫描将跳过本 PR。
Run log: https://github.com/QwenLM/qwen-code/actions/runs/33280928936
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停:本 PR 达到轮次上限(100/100)。评论 |
Address the round-3/4 review of the dirty-worktree pull flows: - A pull that dies without a numeric exit code — the flow's own 30s timeout kill, or an external/OOM signal — may already have written its MERGE_HEAD, so it now counts like a conflicted exit for the abort decision; the tip-identity guard is unchanged, so a merge a terminal parked meanwhile is still left alone. Previously the pull's own merge stayed parked while the response claimed a full restore, and a later commit could finalize the stray merge and silently exclude the upstream's content from history. - The force flow's final ff-only merge is wrapped: a refusal after the discard (e.g. a skip-worktree file the reset cannot clear) is a typed pull_failed instead of raw text the route would re-classify as dirty_working_tree, looping the panel on a discard that can never succeed. - A successful stash pull that had to keep a stash entry (failed drop, slot shift, failed store-back) reports it structurally (stashKept + stashSha); the popover renders that notice as a sticky warning, since it is the only record of where the entries went. - The re-list-failure test pins "HEAD never moved" against a captured headBefore instead of comparing rev-parse with itself.
|
Rounds 3–4 addressed in
Local runs on the pushed head: core On the two red checks on the previous head, for the record: |
|
On the
|
Review + verification report (head 2c8b619)Critical review — no merge-blocking issue found. Full diff read at head; the load-bearing claims check out against the code:
Local verification at head (scratch build from the head tarball,
CI at head: product lanes green (Test ubuntu, Integration no-AK, web-shell smoke, Desktop Shell, Java); mac/win/CLI-integration lanes skipped as normal for fork PRs. Not approving — no maintainer/ci-bot review on this head yet. Automated verification round by qqqys (review-only mandate; local builds at the PR head, no mocks in the e2e stack). |
|
@qwen-code /triage |
qqqys
left a comment
There was a problem hiding this comment.
Re-reviewed the full head at 2c8b619dccf and re-verified every previously reported Critical against the current tree. No blockers found.
The destructive/recovery invariants now hold: auto-stashes are identified by provenance and SHA; restore/drop verifies the SHA actually removed and surfaces a recoverable displaced-entry command; recovery distinguishes pull-created merge/rebase state from a terminal's parked operation; killed pulls take the same safe recovery path; and discard fetches/prunes, validates fast-forwardability, then integrates the exact validated commit rather than a movable upstream ref. Post-discard checkout failures are typed instead of looping back to the dirty-tree panel. The legacy and workspace-qualified routes retain their ownership/trust boundaries, validate mutually exclusive options, and redact git paths on both success notices and typed failures. SDK timeout plumbing and the Web Shell's two-step discard/sticky stash warning match the wire contract.
All current reported CI lanes are green (26 passing; remaining checks are conditional skips). The remaining round-5/6 items are non-blocking follow-ups under the repository's convergence policy. Approving.
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed the full diff with focus on the destructive paths — LGTM, the three by-construction properties hold in code:
Restore by identity, never by position: pushAutoStash identifies its entry via pre/post SHA-set diff + subject match; restoreStash applies by SHA (never stash pop), resolves the slot immediately before drop, verifies the SHA git reports as dropped, and re-stores a displaced entry if the stack shifted — a concurrent terminal push is neither consumed nor blocked.
Validate before discard: forcePull refuses sub-repo-root workspaces via realpath comparison, fetches with --prune, and requires merge-base --is-ancestor HEAD <upstream-tip> before reset --hard + clean -fd; the integration then merges the validated SHA with --ff-only rather than a symbolic ref that could move. Diverged branches and gone upstreams are typed refusals with the tree untouched.
Abort only what the pull started: abortOwnPullState acts only on exit code 1 and proves provenance via tip identity (MERGE_HEAD vs upstream, rebase onto vs upstream from head-name); a parked terminal merge/rebase is left alone, and refuseOperationInProgress (resolved through --git-path, so linked worktrees can't mislead) is re-checked immediately before each state-destroying command.
Route layer: boolean typing + mutual-exclusion validation, and path redaction applied on every response including the success output and the unclassified fall-through. Plain pull is byte-for-byte the previous path.
CI green (Ubuntu tests, no-AK integration, Desktop Shell both OSes, web-shell E2E smoke, visuals, CVE/TruffleHog). Note: the remaining CHANGES_REQUESTED verdicts are stale ci-bot reviews on superseded commits (latest head has 0 unresolved threads).
Code review: dirty-worktree git pull resolution (bd59085..2c8b619)Read-only review of the full diff against the PR description and Verdict: Ready to merge ✅No Critical issues. One Important finding worth addressing; the rest are Minor. Strengths
Important (Should Fix)
Minor (Nice to Have)
Recommendations
|
|
Thanks for the review — sharp catches. Dispositions, now that this is merged:
|
…wenLM#10752) Post-merge review follow-ups for the dirty-worktree pull (QwenLM#10390): - fetchOnly combined with stash or force was silently winning: the flow performed a bare fetch and dropped the resolution the caller asked for. Both combinations are refused now — a 400 invalid_fetch_only_combination at the route and a mirrored guard in gitPull — matching the existing stash+force exclusivity. - A stash or force pull on a detached HEAD surfaced git's raw "HEAD does not point to a branch" through an unclassified 500; it is a typed pull_failed refusal now, before anything is touched. - validatedUpstream resolves the branch once and reads the configured upstream inline, so the no-upstream tail is explicit about the one probe race that can reach it instead of looking unreachable. - The resolution panel no longer offers Discard after the daemon refused it with force_unsupported: the refusal is permanent for a subdirectory workspace, so re-offering it could only loop. - Design doc: note that a stored-back stash entry lands on top of the stack, and state the plain pull's invariant precisely (same git invocation; output now path-redacted like every other response).
…wenLM#10754) * fix(web-shell): disable Push while the branch is behind its upstream Follow-up to QwenLM#10397's sandboxed verification (run 33195566824), which measured two gaps left at merge time: - F1: with an upstream and `behind > 0` — behind-only or diverged — the push row stayed enabled, but the exact `git push` the daemon runs is refused unconditionally as a non-fast-forward, contradicting the derivation's own "disabled means git refuses it" contract. The row is now disabled in those states (the verification's measured M6' rule): `detached || (!operation && hasUpstream && behind > 0)`. Mid-operation the row still only warns — the behind count is in flux until the operation concludes — and conflicts alone still don't block a push. - F2: nothing pinned `newerStatus`'s equal-`computedAt` tie-break (mutant M5 survived), so the popover's own fetch winning ties was unasserted. A test now renders the fetched counters when both stamps are equal. The QwenLM#10390 competing-push panel test gets an ahead-only listing fixture, since its scenario needs a state where push is actually possible. Local mutation A/B: reverting the rule to `detached` fails 3 tests; `>=` → `>` in the tie-break fails exactly the new test. * fix(web-shell): reason about the push destination, not the upstream Review round 1 follow-ups: - The branch listing now carries the push side: `pushTarget` / `pushAhead` / `pushBehind` / `pushGone` from git's own `%(push:short)` and `%(push:track,nobracket)` atoms (no push-destination precedence is re-derived), plus `pushConfigured` (`branch.<name>.pushRemote` or `remote.pushDefault` present). Notably, under the default `push.default=simple` git refuses to resolve `@{push}` in exactly the triangular shape where a plain `git push` succeeds, so the resolved target cannot be the only signal. - The push row's disable and counts now use the push destination: a resolved target brings its own ahead/behind; configured-but-unresolvable fails open (never disables on upstream counts); a missing push ref (`pushGone`) never disables; the plain-clone shape falls back to the upstream, which is where a plain `git push` goes. - The rule-site comment states the counts are as of the last fetch, and all `handlePull` error paths now re-fetch the listing — a pull's embedded fetch has usually updated the refs, so a failed Update no longer strands the rows on a pre-pull snapshot. - The E2E plan's unreachable "click Push while the panel shows" state is rewritten: the reachable check asserts Push is disabled while the 409 panel is up, and the competing-push race keeps its unit coverage on a triangular fixture — a state real git can occupy, unlike ahead-only-with-a-409. Tests: real-git triangular and push-gone fixtures in core (simple → configured-but-unknown, current → resolved counts); derivation cases for the triangular, diverged-from-push-target, push-gone, and configured-but-unresolvable shapes (the last mutation-checked: dropping the fail-open guard fails exactly that test); a pull-failure listing-refetch test. * fix(web-shell): warn instead of disabling when the push counts look doomed Review round 2 measured the behind-based disable misfiring across independent config axes — a `remote.<name>.push` refspec (Gerrit), forcing refspecs, the triangular `push.default=simple` shape, the everyday `checkout -b hotfix origin/main` name-mismatch clone, and plain last-fetch staleness. The common thread: whether a remote will accept a push is not decidable from local state, so every disable built on the counts acquires another carve-out per config axis. This round makes the structural cut instead of the next carve-out: - Push is disabled only on a detached HEAD — the one push failure provable locally. Behind or diverged counts render as warning-tone hints on an enabled row (`↓3`, `↑1 ↓1 · diverged`), and the click surfaces git's own authoritative message. The derivation's contract comment now says exactly that. - The information layer stays push-side and gets honest in the shapes review flagged: a resolved push target brings its own counts; a missing push ref says "Creates <target>" instead of a dimmed "Nothing to push"; a configured-but-unresolvable destination says nothing rather than presenting pull-side numbers as push-side ones. - The `pushConfigured` probe also matches `remote.<name>.push` refspecs, so the Gerrit shape reads as configured rather than as a plain clone. - A rejected push re-reads the listing and the working-tree status (the strongest evidence the counts were stale); a failed pull now refreshes the status alongside the listing so the hints don't mix snapshots. Test hygiene from the same review: the core push-side fixtures run under a hermetic env with `push.default` pinned; new real-git coverage for `remote.pushDefault`, the refspec probe, per-branch case-preserving pushRemote scoping, and a nonzero `pushBehind`; the shared popover fixture is annotated with the wire type (it silently failed to typecheck before); the triangular race fixture uses distinct upstream/push refs (the same-ref contradictory counts were impossible for real git); the pull-failure test asserts the refreshed rows, not just the fetch call; the pushGone test pins its copy. The E2E plan is rewritten for the warn-only semantics with `push.default` made explicit where resolution requires it. * fix(web-shell): key the push row's silence on git naming no destination Review round 3 measured the push row deciding whether it could speak from "a push override is configured" rather than from the boundary its own comment states — git declining to name a destination. Real git shows the key was wrong in both directions: - A tracking upstream whose name the branch does not match under the default `push.default=simple`, and `push.default=nothing`, both leave `%(push)` empty and make a bare `git push` exit 128, yet the row asserted the upstream counts for that refused push (`↑1`, `↑2`). - A branch with no upstream in a repo that sets `remote.pushDefault` lost the accurate "Sets upstream on push" hint, for a push the daemon performs with an explicit refspec that git accepts. Keying the silence on a live upstream with no push destination covers both, and leaves `pushConfigured` with no reader anywhere in the product — so the atom and the `git config --get-regexp` probe that produced it are removed rather than extended. That also retires the probe's serial round-trip on the listing's critical path and the two doc blocks that disagreed about which overrides it detected. The post-action refresh gains a single owner. It is best-effort, so a re-read that fails next to the action that triggered it keeps the stale but usable rows instead of replacing them with its own error; and the pull path no longer awaits it, which had left the resolution panel's buttons disabled for a listing round-trip the panel never needed. Coverage: real-git core fixtures for all three silence shapes, decision table cases for both boundary directions and for the gone-upstream in-sync corner, a push counterpart to the pull-failure refresh test that also pins the status leg, and cases for the best-effort refresh and the responsive panel. Fixtures that meant to exercise the count path now carry the push-side atoms core actually emits. Every guard added here was mutation probed; the E2E plan gains the two states the re-keyed boundary changes. * fix(web-shell): keep the stale rows on screen through a post-action refresh Review round 5 measured the silent refresh added in round 3 defeating the property it was added for. `fetchBranches(true)` raised the same `loading` flag the on-open fetch does, and the render gate swaps the whole row set — action rows and branch sections alike — for the "Loading branches…" placeholder while that flag is set. So after a rejected push or a failed Update Project the listing was replaced for the full daemon round-trip, exactly the window its own comment calls "stale but usable": against the correlated failure it exists for (a closing daemon generation, with the SDK's 30s fetch timeout) the user faced a blank list instead of clickable rows. The push-side `await`'s stated purpose — holding the row spinner up until the refresh lands — was likewise unobservable, because the row carrying that spinner was not mounted. Suppressing the toggle for silent refreshes only keeps the placeholder on the on-open path the gate is fed by. The same round asked whether the push-failure re-read heals the counts that got the push rejected. Real git says it cannot: a non-fast-forward rejection moves no local ref, so the listing and the status read both come back byte-identical (`[ahead 1]` before and after the rejected push; `[ahead 1, behind 1]` only once something fetches). Adding a fetch to make it heal was declined rather than worked around — the design already makes git's click-time message the authority on remote acceptance, that message reaches the status line from the same catch, the only fetch timeout in the component is the pull path's 600s, and a reconciliation effect already re-reads the listing when a newer status contradicts it. The rule-site comment and the E2E plan's push bullet now claim a re-read and name git's message as the authority, so the copy stops promising a healing this path cannot deliver. Coverage: one post-rejection test holding the second listing promise pending witnesses both guards — the rows stay mounted with no placeholder while it is in flight, and the push row stays disabled until it settles. Mutation probed: dropping `!silent` fails it on the placeholder assertion, flipping the push-side `await` to `void` fails it on the disabled one, restored control green. --------- Co-authored-by: qwen-code-dev-bot <[email protected]>


What this PR does
The workspace "Update Project" action in the Web Shell now handles a dirty working tree instead of dead-ending on it. When a plain pull is blocked by uncommitted changes, the branch picker's footer switches from an opaque one-line error to a resolution panel with two ways forward: Stash Changes and Update (stash the local changes including untracked files, pull, restore them) or Discard Changes and Update… (behind a second confirming click). Cancelling dismisses the panel; reopening the picker or starting any other action resets it.
The pull endpoint accepts two opt-in booleans,
stashandforce(mutually exclusive, both off by default). They do exactly what a user would do in a terminal, and the repository is always left in a known state:git stash push --include-untracked, the samegit pullas before, then restore. The entry is identified by SHA (refs/stashcompared before/after the push,git stash apply <sha>, drop by slot looked up at drop time), never by stack position, so a terminal pushing to the shared stash meanwhile is neither consumed nor in the way. If the pull fails, the merge or rebase it started is aborted and the entry is restored →409 pull_failedwith git's message. If the restore itself conflicts, the response is a success withstashRestoreConflict: true; git keeps the entry and the output names it.git reset --hard+git clean -fd(ignored files kept), thengit pull --ff-only. Validated before anything is destroyed: fetch first, refuse a diverged branch (409 diverged) while the local changes are still intact, so the post-discard pull can only ever fast-forward. Refused from a workspace below the repository root (409 force_unsupported), becausereset --hardwould also erase changes outside the workspace.409 operation_in_progress) while a merge, cherry-pick, revert, rebase or am is parked in the worktree, sincestash pushandreset --hardboth clear that state; the failure recovery therefore only ever aborts what the pull itself started.A plain pull with no option is byte-for-byte the previous behavior. The SDK's
workspaceGitPullgains the two options, thestashRestoreConflictresult field, and a per-call timeout so the popover can outsize the client's default fetch budget for the multi-command flows.Replaces #9769. That PR started as this same 743-line feature and grew to +8217/−228 over 18 autofix rounds, each adding a preflight or guard (ignored-file collision probe, per-repo pull lock, identity re-verification, pinned merge flags overriding the user's git policy, hermetic-config shields in tests) whose edge cases the next review round then found — 235 review threads, all unresolved, and the bot itself asking for the PR to be reduced. This PR keeps the three properties that are closed by construction (restore by identity, validate before discard, abort only what we started) and records the rest as explicit non-goals in
docs/design/git-pull-dirty-worktree.md: ambient git configuration is honored as in a terminal, ignored files are expendable as in git, and concurrent pulls fail loudly on git's own index lock rather than being serialized. Net: +1627/−77 across the same 12 files, coregit-branches.ts+301 lines instead of +1362.Why it's needed
Users who keep uncommitted work in a workspace could not use the Web Shell git update at all: the pull was refused and the UI only rendered the raw daemon error code, forcing everyone back to a terminal to stash or clean by hand. The workspace list already knows the working tree state for its git chip, so surfacing the two standard resolutions inline closes the loop without leaving the shell.
Reviewer Test Plan
How to verify
Automated coverage runs every layer against real git repositories (bare remote + workspace clone + a second clone standing in for another developer); all targeted runs pass locally:
cd packages/core && npx vitest run src/utils/git-branches.test.ts, 72 passed, 15 new): plain dirty pull still refused by git unchanged; stash round trip restores tracked edits and untracked files with an empty stash list; nothing-to-stash is a plain pull; stash + rebase replays the local commit linearly; conflicting restore keeps the entry and reports its SHA; failed merge and failed rebase both restore the exact pre-pull state (HEAD, no MERGE_HEAD/rebase dir, edits, untracked file, empty stash); a foreign stash pushed mid-pull (via a realpost-mergehook) is left untouched while ours is applied and dropped by identity; missing upstream fails before stashing; force discards tracked/untracked and keeps ignored; force refuses a diverged branch and a subdirectory cwd before discarding; stash+force throws; merge-in-progress and stopped-rebase refuse both flows and keep the state.cd packages/cli && npx vitest run src/serve/routes/workspace-git-branches.test.ts, 32 passed, 10 new): wrong-typed / combined options → 400; dirty plain pull → 409dirty_working_treepath-redacted; stash → 200 with changes restored; conflicting restore → 200 +stashRestoreConflict; recovered failure → 409pull_failedpath-redacted with MERGE_HEAD gone; force → 200; diverged force → 409divergedwith edits intact; in-progress merge → 409operation_in_progress.cd packages/web-shell && npx vitest run client/components/BranchPickerPopover.test.tsx, 10 passed, 7 new; full web-shell suite 4401 passed): panel on dirty 409, stash click →{ stash: true }with the 300 s timeout, discard needs the confirm click before{ force: true }, panel stays mounted (button disabled) while in flight,stashRestoreConflictrenders as a warning, other refusals render the daemon message without the panel, Cancel sends no request, reopen resets.cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts, 353 passed, 2 new): per-call timeout overrides the client budget on both routes and stays out of the JSON body.Real stack, no mocks:
npm run bundle→node dist/cli.js serve --workspace <fixture>with an isolatedQWEN_HOME, driven by Playwright in Chromium with a/git/pullrequest ledger, then the repository inspected with git after each scenario. Fixture: 20-line README, upstream edits line 1; workspace edits line 20 (clean-restore) or line 1 (conflict-restore), plus an untrackednotes.txtand an ignoreddist/out.txt.{}→ 409dirty_working_tree{"stash":true}→ 200notes.txtback;dist/out.txtkept;git stash listempty{"stash":true}→ 200 +stashRestoreConflictUUwith conflict markers;stash@{0}: qwen-code: auto-stash before pullstill carries+line 1 (local WIP);notes.txtback{"force":true}→ 200notes.txtgone;dist/out.txtkept?lang=zh-CNEvidence (Before & After)
main, from the #9769 verification run)POST /workspaces/:workspace/git/p…, truncated, no actions.zh-CN:
Tested on
Environment (optional)
macOS, Node 24.18.1, git 2.55.0. Unit and integration tests against real local git repositories; real
qwen servedaemon fromnpm run bundle+ Playwright/Chromium for the browser evidence.npm run typecheckis clean for every package touched (the only failures in this worktree are the pre-existing ajv-version errors inintegrations/external-context-mem0, unrelated to this change); eslint clean on all changed files.Risk & Scope
git pulland to the plain pull onmaintoday), no override of the user'spull.rebase/pull.ff/autostash policy, no cross-request pull serialization. The route's pre-existing text-based classification of plain-pull errors is unchanged.stashRestoreConflictand the SDK timeout parameter are additive.Linked Issues
Replaces #9769 (same feature, reduced to a closed design; see the cross-reference comment there).
中文说明
这个 PR 做了什么
Web Shell 中工作区的「更新项目」操作现在能处理脏工作区,而不是遇到它就卡死。当裸 pull 被未提交的修改阻塞时,分支选择器底部会从一行看不懂的错误切换为一个选项面板,提供两种处理方式:Stash 修改并更新(把本地修改含未跟踪文件 stash 起来、pull、再恢复)或 放弃修改并更新…(需要第二次点击确认)。取消会收起面板;重新打开选择器或执行任何其它操作都会重置它。
pull 接口新增两个可选布尔值
stash与force(互斥,默认都关)。它们做的正是用户在终端里会做的事,并且仓库始终处于已知状态:git stash push --include-untracked,跑与之前完全相同的git pull,再恢复。stash 条目按 SHA 识别(push 前后比对refs/stash、git stash apply <sha>、drop 时再查槽位),从不按栈顶位置——终端在此期间往共享 stash 里 push 既不会被误消费也不会挡路。pull 失败时中止它自己发起的 merge/rebase 并恢复条目 →409 pull_failed附 git 的说明。恢复本身冲突时,响应仍为成功但带stashRestoreConflict: true;git 保留条目,output 里点名。git reset --hard+git clean -fd(保留 ignored 文件),再git pull --ff-only。破坏性操作前先校验:先 fetch,分叉分支在本地修改尚未丢弃时就拒绝(409 diverged),因此丢弃后的 pull 只可能快进。工作区位于仓库根目录之下时拒绝(409 force_unsupported),因为reset --hard会连工作区之外的修改一起抹掉。409 operation_in_progress),因为stash push与reset --hard都会清掉这些状态;失败恢复因此只会中止 pull 自己发起的操作。不带选项的裸 pull 与之前逐字节一致。SDK 的
workspaceGitPull新增两个选项、stashRestoreConflict结果字段,以及按调用指定的超时,让弹窗为多命令流程放大客户端默认预算。替代 #9769。 那个 PR 起点就是这个 743 行的特性,经 18 轮 autofix 膨胀到 +8217/−228,每轮加一层预检或护栏(ignored 文件碰撞探针、按仓库的 pull 锁、身份复核、覆盖用户 git 策略的固定 merge 参数、测试里的 hermetic 配置盾),而下一轮评审又找出它们的边角——235 条评审线程全部未 resolve,bot 自己也要求缩减 PR。本 PR 只保留三条构造上闭合的性质(按身份恢复、先校验再丢弃、只中止自己发起的操作),其余在
docs/design/git-pull-dirty-worktree.md里明确记为非目标:像终端一样尊重环境 git 配置;ignored 文件按 git 语义视为可牺牲;并发 pull 靠 git 自己的 index lock 大声失败而不做串行化。净变化:同样 12 个文件 +1627/−77,core 的git-branches.ts+301 行而非 +1362。为什么需要
工作区里有未提交修改的用户完全无法使用 Web Shell 的 git 更新:pull 被拒绝,而 UI 只显示原始错误码,用户只能回到终端手动 stash 或清理。工作区列表的 git 徽标本就掌握工作区状态,把两种标准处理方式直接呈现在界面上,可以不离开 Web Shell 完成闭环。
评审测试计划
如何验证
自动化覆盖逐层针对真实 git 仓库(裸远端 + 工作区 clone + 扮演另一位开发者的第二个 clone)运行,本地定向测试全部通过:
cd packages/core && npx vitest run src/utils/git-branches.test.ts,72 通过,新增 15):脏树裸 pull 仍由 git 拒绝且行为不变;stash 往返恢复已跟踪修改与未跟踪文件且 stash 列表为空;无可 stash 内容时等同裸 pull;stash + rebase 线性回放本地提交;恢复冲突时保留条目并报告其 SHA;merge 失败与 rebase 失败都恢复到精确的 pull 前状态(HEAD、无 MERGE_HEAD/rebase 目录、修改、未跟踪文件、空 stash);pull 途中被推入的外来 stash(用真实post-merge钩子制造)原样保留,而我们的条目按身份 apply+drop;缺 upstream 在 stash 前就失败;force 丢弃已跟踪/未跟踪并保留 ignored;force 在分叉分支与子目录 cwd 上丢弃前即拒绝;stash+force 抛错;进行中的 merge 与停住的 rebase 让两种流程都拒绝且状态保留。cd packages/cli && npx vitest run src/serve/routes/workspace-git-branches.test.ts,32 通过,新增 10):类型错误/组合选项 → 400;脏树裸 pull → 409dirty_working_tree且路径脱敏;stash → 200 且修改恢复;恢复冲突 → 200 +stashRestoreConflict;已恢复的失败 → 409pull_failed路径脱敏且 MERGE_HEAD 已清;force → 200;分叉 force → 409diverged且修改完好;进行中的 merge → 409operation_in_progress。cd packages/web-shell && npx vitest run client/components/BranchPickerPopover.test.tsx,10 通过,新增 7;web-shell 全套 4401 通过):脏树 409 出面板,点 stash 以{ stash: true }与 300 s 超时调用,放弃需先确认才发{ force: true },请求在途时面板保持挂载(按钮禁用),stashRestoreConflict渲染为警告,其它拒绝显示 daemon 消息且不出面板,取消不发请求,重开重置。cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts,353 通过,新增 2):按调用的超时在两条路由上都覆盖客户端预算,且不进入 JSON body。真实栈、无 mock:
npm run bundle→node dist/cli.js serve --workspace <夹具>配隔离QWEN_HOME,Playwright/Chromium 驱动并记录/git/pull请求台账,每个场景后用 git 检查仓库。夹具:20 行 README,上游改第 1 行;工作区改第 20 行(干净恢复)或第 1 行(冲突恢复),另有未跟踪notes.txt与 ignored 的dist/out.txt。{}→ 409dirty_working_tree{"stash":true}→ 200notes.txt回来;dist/out.txt保留;git stash list为空{"stash":true}→ 200 +stashRestoreConflictUU带冲突标记;stash@{0}: qwen-code: auto-stash before pull仍含+line 1 (local WIP);notes.txt回来{"force":true}→ 200notes.txt消失;dist/out.txt保留?lang=zh-CN前后对比证据
main,取自 #9769 的验证运行)POST /workspaces/:workspace/git/p…,被截断,没有任何操作。zh-CN:
测试环境
环境(可选)
macOS,Node 24.18.1,git 2.55.0。单元与集成测试针对真实本地 git 仓库;浏览器证据来自
npm run bundle起的真实qwen servedaemon + Playwright/Chromium。所有触及的包npm run typecheck干净(本 worktree 仅integrations/external-context-mem0有既存的 ajv 版本错误,与本改动无关);改动文件 eslint 干净。风险与范围
git pull及当前main的裸 pull 一致)、不覆盖用户的pull.rebase/pull.ff/autostash 策略、不做跨请求 pull 串行化。路由里既有的按文本分类裸 pull 错误的逻辑未变。stashRestoreConflict与 SDK 超时参数都是增量。关联 Issue
替代 #9769(同一特性,收敛为闭合设计;见该 PR 上的交叉引用评论)。