mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
7281f0de8c
* feat(core): add working_dir to the Agent tool for pinning subagents to an existing worktree
Add an opt-in working_dir parameter to the Agent (sub-agent) tool that pins a sub-agent's entire working-directory context to an existing, caller-owned git worktree of the current repository. Unlike isolation: 'worktree', the harness neither provisions nor removes the directory — the caller owns its lifecycle — so a sub-agent can be aimed at a worktree that some earlier step already prepared.
Every "where am I?" surface on the sub-agent's config (target dir, cwd, project root, file discovery, workspace context) is rebound to the worktree, reusing the existing worktree-isolation rebind, so the sub-agent's file and shell tools operate inside that directory and cannot leak into the parent project tree. The path is validated as a worktree registered against the current repository before use, and working_dir is mutually exclusive with isolation.
Wire the /review skill to pass working_dir to every review agent for same-repo PR reviews, so the PR worktree isolation is enforced deterministically at the agent boundary instead of by prompt convention. This makes reviewing multiple PRs concurrently safe.
* fix(core): harden working_dir validation from review feedback
- Reject a working_dir that resolves to the repository's own main working tree. getRegisteredWorktreeBranch accepts the primary checkout too, so `working_dir: "."` or the repo root would otherwise pin a sub-agent to the user's main tree and silently defeat the isolation. A linked worktree has a `.git` file; the main tree has a `.git` directory — require the former.
- Reject working_dir combined with run_in_background. The caller owns the worktree lifecycle and could remove it while a detached agent is still running in it (ENOENT on its own cwd); the isolation: 'worktree' path does not have this problem because the tool owns and reaps the worktree.
- Add an isGitRepository() preflight so a non-repo parent reports the real cause instead of a misleading "not a registered git worktree" error.
- Add tests: repo-relative working_dir resolution (the /review production form), main-working-tree rejection, and the run_in_background guard.
* fix(core): make main-worktree detection robust and tighten working_dir validation
Address a second review round on the Agent tool working_dir parameter.
- Replace the ".git is a file ⟹ linked worktree" heuristic with a reliable check: a linked worktree's per-worktree git dir (--absolute-git-dir) differs from the common git dir (--git-common-dir), while a main working tree's are identical. The heuristic was wrong for a main tree whose .git is a file (git clone --separate-git-dir, submodules), which would have let the main checkout pass the isolation guard. Extracted as GitWorktreeService.isLinkedWorktree(); the fs.stat probe (and its all-errors catch) is gone.
- Reject whitespace-only working_dir (trim before the empty check), matching the worktree-name validation.
- Tests: assert the sub-agent's getCwd()/getWorkingDir() are rebound (not just project root / target dir); cover the monorepo-subdirectory re-anchor branch (getTargetDir() below the repo root); and add real-git coverage for isLinkedWorktree, including the separate-git-dir main tree.
* fix(core): reject working_dir for named teammates
A named teammate spawns via TeamManager with cwd = getCwd() and returns before the working_dir rebind is reached, so the pin was silently ignored and the teammate ran in the parent working tree — a false sense of isolation. Reject working_dir at the point a teammate would actually spawn (name + active TeamManager + top-level session), leaving the name-without-active-team fallback (which spawns a normal sub-agent, where working_dir does apply) untouched. Add a test.
* docs(core): describe working_dir as a cwd pin, not a sandbox
The working_dir parameter description, the interface doc, and the /review skill said the sub-agent "cannot touch"/"cannot leak into" the parent tree. That overstates it: file and shell tools still accept absolute paths, so the isolation is a working-directory pin (cwd-relative operations and search tools stay inside the worktree), not a filesystem sandbox — the same limitation as isolation: 'worktree'. Reword all three to state this accurately.
* test(core): cover working_dir non-git and fail-closed paths; clarify main-tree error
- Add an execute-level test for the working_dir isGitRepository() preflight (non-git parent directory -> "not a git repository").
- Add a test for isLinkedWorktree's fail-closed catch (a non-git path returns false).
- Reword the main-tree rejection error to name both possible causes ("is the main working tree, or its git metadata could not be read"), since isLinkedWorktree also fails closed on a git error rather than only on the main tree.
* fix(core): canonicalize path in isLinkedWorktree; test relative working_dir in a monorepo
- isLinkedWorktree now realpaths the worktree path before comparing --absolute-git-dir (which git returns canonicalized) against --git-common-dir. Without this, a symlinked input (macOS /var → /private/var, os.tmpdir()) made the two diverge for the main working tree and misreported it as linked, letting the main checkout pass the isolation gate.
- Add a test proving a repo-relative working_dir resolves against the subdirectory cwd where fetch-pr actually creates the worktree (git resolves relative worktree paths against cwd), not the repo root — the production monorepo form.
* fix(core): close working_dir gaps from review — classifier, background config, review-workflow coverage
- Include working_dir in toAutoClassifierInput so AUTO-mode permission classification can see the child is rebound to another worktree; a launch could otherwise look benign from subagent_type + prompt alone.
- Reject working_dir when the effective background decision is true — not only for the explicit run_in_background param (already rejected in validateToolParams) but also for a subagent config with background: true, which validateToolParams cannot see. Guard on the resolved shouldRunInBackground so a nested call that downgrades to the foreground is not over-rejected.
- /review skill: extend the working_dir mandate to the Step 4 verification agent and Step 5 reverse-audit agents, which also read files and re-check the diff and would otherwise run against the user's main checkout.
- Add tests for all three.
* test(core): symlink + preserved-suffix coverage; soften working_dir grep/glob wording
- isLinkedWorktree: add a test that passes a symlinked path (the macOS /var → /private/var case, reproduced with an explicit symlink) so the realpath canonicalization is actually exercised.
- Assert a caller-owned worktree run does not emit a spurious "[worktree preserved]" suffix, guarding the externallyManaged cleanup skip against regression.
- Soften the working_dir description: grep/glob default to the worktree as their root but can still be pointed outside via an explicit path, so drop the "enforce it as the workspace boundary" claim.
* fix(core): validate working_dir against the authoritative git worktree registry
- Replace the --git-dir vs --git-common-dir "is it a linked worktree" heuristic with an authoritative check against `git worktree list`. The heuristic could be fooled by a hand-crafted directory carrying a .git file copied from a real linked worktree (structurally a linked worktree, yet not registered); `git worktree list` reads .git/worktrees/<name>/gitdir and excludes such a fake. Renamed isLinkedWorktree -> isRegisteredLinkedWorktree.
- Tests: reject a fake worktree (copied .git file); match a registered worktree through a symlinked input path; and a nested-context test confirming a background: true subagent that downgrades to foreground is NOT rejected by the working_dir background guard (guards the shouldRunInBackground keying against regression).
* fix(core): fix flaky nested working_dir test; harden worktree-registry parsing
- The nested background:true test relied on the default '/test/project' cwd, which does not exist on CI, so simple-git threw at construction instead of the test reaching the isGitRepository() path (this is the red CI on the prior commit). Point it at a real, non-git temp directory instead.
- isRegisteredLinkedWorktree: parse `git worktree list --porcelain -z` (NUL-terminated) so a worktree path that itself contains a newline cannot inject a fake `worktree <path>` entry into a newline-split parse; and reject the primary working tree by canonical path so a linked record whose on-disk path is a symlink onto the main checkout cannot smuggle it through.
* fix(core): fall back when `worktree list -z` is unsupported; add newline-injection regression test
- `git worktree list -z` requires Git >= 2.36. On older git the call threw and the surrounding catch rejected every valid caller-owned worktree with a generic "not a registered linked worktree" error. Fall back to the newline-separated porcelain form instead of hard-failing: that parse is only vulnerable to a worktree path that itself contains a newline, which is a far narrower exposure than rejecting every worktree.
- Add a real-git test that creates a worktree whose path embeds "\nworktree <other>" and asserts the injected path is not accepted while the real (awkwardly named) worktree still validates. Confirmed the test fails when the parser reverts to newline splitting, so it genuinely guards the -z parse.
* fix(core): accept detached-HEAD worktrees for working_dir; cover the failure-path cleanup guard
- getRegisteredWorktreeBranch returns null for a detached-HEAD worktree (its branch reads as 'HEAD'), so a legitimate `git worktree add --detach` target was rejected with a factually wrong "not a registered git worktree" error and the authoritative registry check never ran. Make isRegisteredLinkedWorktree the sole gate and read the branch best-effort: branch is unused for caller-owned worktrees anyway, since cleanup short-circuits on externallyManaged.
- Add a test asserting a detached worktree is accepted and the sub-agent is rebound onto it.
- Add a failure-path test: when the sub-agent throws, the finally / outer-catch path runs cleanupWorktreeIsolation() and the externallyManaged guard must leave the caller's worktree intact. Confirmed both new tests fail without their respective fixes, so they genuinely guard the behaviour.
* fix(core): verify a worktree's registry pointer instead of parsing `git worktree list`
Parsing the registry list forced a bad trade: `--porcelain` alone is newline-delimited, so a worktree path containing a newline can inject a bogus entry, while `-z` needs Git >= 2.36 and would reject every worktree on older git. Verify the single path in question instead:
1. `--git-dir` equal to `--git-common-dir` means the primary working tree, which is rejected.
2. The common dir must be this repository's, or the path belongs to another repo.
3. `<gitDir>/gitdir` records the worktree that registry entry belongs to; it must resolve back to the probed path.
Step 3 is what makes it authoritative: a directory holding a `.git` file copied from a real linked worktree does report a per-worktree git dir, but that entry's pointer names the real worktree, not the copy. No list is parsed, so the newline-injection class of bug is gone on every git version and no `-z` (Git >= 2.36) is required. Detached worktrees, symlinked inputs, separate-git-dir main trees, and other repositories' worktrees are all still handled correctly.
Also bump the real-git integration suite's vitest timeouts to 30s, matching the sibling hooks/symlinks suites, to reduce CI flakiness.
* fix(core): contain working_dir to the repo, tailor the pinned-worktree notice, keep the newline test off Windows
- Containment: a registered worktree can live anywhere on disk, and pinning rebinds the child's WorkspaceContext wholesale, so a model-supplied path could silently move the file tools' boundary outside the repository. Require the resolved path to sit inside the repo (canonical comparison, so a symlink cannot straddle the boundary) and say so in the parameter description. isolation: 'worktree' already had this implicitly.
- Prompt: a caller-owned worktree is the code the agent was asked to work on, not a provisioned copy of the parent's tree. Give it a narrow notice instead of the isolation notice, whose "translate the parent's paths" and "re-read what the parent changed" guidance contradicts the caller's own instructions (/review tells its agents not to cd or prefix absolute paths).
- Skip the newline-injection test on Windows: a path component containing a newline is not representable there (the embedded drive letter makes it invalid), so `git worktree add` errors and the windows test job would fail. The injection cannot occur on Win32 either.
- Add a regression test for a stale registry record whose directory was recreated as a plain directory: `git worktree list` keeps such records for months, and `git rev-parse --show-toplevel` inside one resolves to the MAIN checkout. Probing the path itself rejects it.
* docs(core): correct the working_dir helper's doc comment
The comment still credited getRegisteredWorktreeBranch with enforcing "must be a registered worktree". That gate is now isRegisteredLinkedWorktree (plus the in-repository containment check); getRegisteredWorktreeBranch is consulted only for a best-effort branch label and is deliberately not a gate, since it returns null for a legitimate detached-HEAD worktree.
* fix(core): read the worktree registry from the repo, not the candidate directory
isRegisteredLinkedWorktree derived everything from files inside the directory being validated: `<target>/.git` names a git dir, whose `commondir` and `gitdir` were then trusted. All three are candidate-controlled, so a *fabricated* chain (not merely a `.git` copied from a real worktree) could name itself and be accepted even though git's registry holds no entry for it. The docstring's claim that it consulted "git's own registry entry" was therefore wrong.
Read the registry from the repository instead: some `<commonDir>/worktrees/<name>/gitdir` must name the path. Keep a liveness probe inside the path as well: its own `--git-dir` must be that same entry. The two are complementary. The registry answers "is this path registered?" and so rejects a fabricated chain, a copied `.git` file (the entry names the original), another repository's worktree, and the primary working tree, which has no entry of its own. Only the probe answers "is it a worktree right now?" and so rejects a stale `prunable` record whose directory was deleted and recreated as an ordinary directory.
Add a fabricated-chain regression test, and confirm each of the two tests fails when its corresponding half of the check is removed.
|
||
|---|---|---|
| .. | ||
| scripts | ||
| src | ||
| vendor | ||
| index.ts | ||
| package.json | ||
| test-setup.ts | ||
| tsconfig.json | ||
| vitest.config.ts | ||