fix(cases): clone into the chosen folder itself, not a subfolder of it

The custom folder was treated as a parent, so the repository landed in <folder>/<case name>. The chosen folder is now the clone destination: an empty existing folder is cloned into directly (CloneOptions.intoEmptyDir, no temporary sibling, never removed on failure), a new folder is created, and a folder with files is refused.
This commit is contained in:
Devvyn committed 2026-10-31 16:43:11 +08:00
1 parent 0bd15ddc84
commit 68bd7e4d15
8 files changed
+93 -43

No files matched your search

+1 -1
View File
@@ -244,7 +244,7 @@ The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLin
**Ultracode / workflow-run visualization** (opt-in, default OFF): the Workflow tool writes a completion artifact only at run *end*, so live in-flight runs exist solely as transcript dirs. `workflow-run-watcher.ts` therefore synthesizes ACTIVE runs from transcripts until the completion artifact appears and supersedes them. It is **STANDALONE** and deliberately never imports or touches `subagent-watcher.ts`, despite reading the same tree. Two independent toggles: `showUltracodeAgents` (docked panel) and `ultracodeFloatingWindows` (floating windows); the watcher starts if **either** is on. → [architecture-invariants#ultracode-and-workflow-run-visualization](docs/architecture-invariants.md#ultracode-and-workflow-run-visualization)
**Clone a repository as a case** (issue #236, Add Case → **Clone Repo**): `POST /api/cases/clone` clones synchronously into the caller's case space (bounded by `GIT_CLONE_TIMEOUT_MS`, no job store); `POST /api/cases/clone-preflight` checks anonymous cloneability and lists refs. Core in `src/git-clone.ts`. ⚠️ **The URL is a code-execution surface**: refuse every `::` form and a leading `-`, spawn only argv arrays with `--` before operands. ⚠️ Stay non-interactive (`gitNonInteractiveEnv()`) or the open request hangs; never collect credentials, refuse `user:password@` URLs. ⚠️ Timeout kills the process GROUP, remove the destination only if this attempt created it, and repo contents win over scaffolding (existing `CLAUDE.md` kept, hooks merged, repo `.claude/settings*` warned about). ⚠️ A `path` clones into a custom folder and registers a linked case: admin-only in multi-user mode, judged by `prepareNewCasePath()`, and an existing target (even empty) is refused. → [architecture-invariants#clone-a-repository-as-a-case](docs/architecture-invariants.md#clone-a-repository-as-a-case)
**Clone a repository as a case** (issue #236, Add Case → **Clone Repo**): `POST /api/cases/clone` clones synchronously into the caller's case space (bounded by `GIT_CLONE_TIMEOUT_MS`, no job store); `POST /api/cases/clone-preflight` checks anonymous cloneability and lists refs. Core in `src/git-clone.ts`. ⚠️ **The URL is a code-execution surface**: refuse every `::` form and a leading `-`, spawn only argv arrays with `--` before operands. ⚠️ Stay non-interactive (`gitNonInteractiveEnv()`) or the open request hangs; never collect credentials, refuse `user:password@` URLs. ⚠️ Timeout kills the process GROUP, remove the destination only if this attempt created it, and repo contents win over scaffolding (existing `CLAUDE.md` kept, hooks merged, repo `.claude/settings*` warned about). ⚠️ A `path` is the folder cloned INTO (no case-name subfolder) and registers a linked case: admin-only in multi-user mode, judged by `prepareNewCasePath()`, empty or new only (`intoEmptyDir` clones straight into an empty one and never removes it). → [architecture-invariants#clone-a-repository-as-a-case](docs/architecture-invariants.md#clone-a-repository-as-a-case)
**Git worktrees of a case** (case picker → **Git worktrees…**; `src/git-worktrees.ts`, `routes/worktree-routes.ts`): a local repository case keeps worktrees at `<repo>/.claude/worktrees/<name>` on `worktree-<name>`, Claude Code's own `--worktree` layout. ⚠️ Only that managed path is ever created or removed, every component `lstat`ed and realpath-checked (a symlinked `.claude` is refused). ⚠️ git never runs for a Docker or remote case, nor at or inside a Docker workspace (`worktree add` runs hooks and filters on the host). ⚠️ Remove is `git worktree remove` WITHOUT `--force`, refused while a live session is inside; branch delete is `-d`, never `-D`. ⚠️ Sessions start through the existing create paths (quick-start `worktree`, or `POST /api/sessions` `workingDir`), never a new spawn path. → [architecture-invariants#git-worktrees-of-a-case](docs/architecture-invariants.md#git-worktrees-of-a-case)
+1 -1
View File
@@ -468,7 +468,7 @@ Tests: `test/file-preview-markdown.test.ts` (jsdom-in-vm, pins every rule above)
### Clone a repository as a case
**Clone Repo tab** (issue #236, proposed by @DodgyBadger): `POST /api/cases/clone` clones a public repository into the caller's case space and registers it as a normal local case; `POST /api/cases/clone-preflight` answers "can this be cloned anonymously, and what refs does it have?" while the user is still typing. Core in `src/git-clone.ts`, split into a PURE half (URL parse, argv/env, `ls-remote` parse, stderr classification) and a thin IO half (`probeGitRemote`, `cloneRepository`). An optional `path` (the **Clone into a custom folder** checkbox, parent folder plus case name) clones into that exact folder instead and registers it as a LINKED case, the same way `POST /api/cases` with a `path` does: it is judged by `prepareNewCasePath()` (`web/case-path.ts`: blocked trees, symlinks, parent must exist), is admin-only in multi-user mode (it writes an absolute path into the shared linked-cases registry), and refuses a folder that already exists, empty or not, because `cloneRepository` needs a fresh destination. Deleting such a case unlinks it and never removes the files. If the registry write fails after git finished, the folder is kept and the error says where it is.
**Clone Repo tab** (issue #236, proposed by @DodgyBadger): `POST /api/cases/clone` clones a public repository into the caller's case space and registers it as a normal local case; `POST /api/cases/clone-preflight` answers "can this be cloned anonymously, and what refs does it have?" while the user is still typing. Core in `src/git-clone.ts`, split into a PURE half (URL parse, argv/env, `ls-remote` parse, stderr classification) and a thin IO half (`probeGitRemote`, `cloneRepository`). An optional `path` (the **Clone into a custom folder** checkbox) is the folder the repository is cloned INTO, exactly as given (the case name adds no subfolder), and it registers a LINKED case, the same way `POST /api/cases` with a `path` does: it is judged by `prepareNewCasePath()` (`web/case-path.ts`: blocked trees, symlinks, parent must exist), is admin-only in multi-user mode (it writes an absolute path into the shared linked-cases registry), and must be empty or not exist yet. An existing empty folder is cloned into directly (`CloneOptions.intoEmptyDir`: no temporary sibling and no rename, so a mount point works; a failed attempt empties it again but never removes it); a folder with anything in it is refused. Deleting such a case unlinks it and never removes the files. If the registry write fails after git finished, the folder is kept and the error says where it is.
- ⚠️ **The URL is a code-execution surface, which is why it is parsed rather than forwarded.** `ext::sh -c <cmd>` makes git run an arbitrary command as its transport, and ANY `<name>::<payload>` dispatches to a `git-remote-<name>` helper, so every `::` form is refused outright. A repository starting with `-` is read by git as a flag; that is rejected AND every spawn puts `--` before the operands, because either defence alone is one edit away from being a hole. Spawns are argv arrays, never a shell (unlike `remote-hosts.ts`, which does build a shell line and must `shellescape`). The Zod schema deliberately only length-bounds `repository` — a weaker regex duplicate of `parseGitRepositoryUrl` would be the copy that drifts.
- ⚠️ **Non-interactive or it hangs the request.** The clone is synchronous by design (no job store, no polling, no cancellation surface), so an invisible credential prompt would pin an open HTTP request until the timeout. `gitNonInteractiveEnv()` closes all four prompt paths at once: `GIT_TERMINAL_PROMPT=0`, empty `GIT_ASKPASS`/`SSH_ASKPASS` + `SSH_ASKPASS_REQUIRE=never` + empty `DISPLAY`, `GCM_INTERACTIVE=never`, and `ssh -oBatchMode=yes`. `HOME`/`PATH` are inherited on purpose — a user whose own agent or credential helper already works keeps working (so a private repo may well clone; Codeman just never collects or stores credentials, and refuses a `user:password@` URL). ⚠️ In multi-user mode a NON-ADMIN's clone and preflight run with `git -c credential.helper=` (`cloneWithoutCredentialHelpers`), because the helpers belong to the one server account every user shares; admins and single-user mode keep them. The Docker server image relies on exactly this: when built with the opt-in `CODEMAN_INSTALL_GH`/`CODEMAN_INSTALL_AZ` args it configures `gh` (github.com) and an `az`-backed helper (`docker/git-credential-azure-cli`, Azure DevOps) in its SYSTEM gitconfig, both silent until the user signs the CLI in, so a private clone there still fails fast rather than prompting.
+1 -1
View File
@@ -25,7 +25,7 @@ Three ways to get one, all under **New or link a case…** at the bottom of the
| How | Result |
| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **Create New** | A fresh `~/codeman-cases/<name>` with a scaffolded `CLAUDE.md`, or with **Create in a custom folder**, a new folder inside a parent you choose, scaffolded the same way and registered in place like a linked case. |
| **Clone Repo** | A repo cloned into `~/codeman-cases/<name>` (or into a folder you choose) and registered as a case. Private repos need this machine's own git credentials (see below). |
| **Clone Repo** | A repo cloned into `~/codeman-cases/<name>` (or straight into an empty folder you choose) and registered as a case. Private repos need this machine's own git credentials (see below). |
| **Link Existing** | An existing folder anywhere on disk, registered in place. Nothing is copied or moved. |
Linked cases keep living where they are. Deleting a case in Codeman removes the
+39 -6
View File
@@ -50,7 +50,7 @@
import { spawn, execFileSync } from 'node:child_process';
import { randomBytes } from 'node:crypto';
import { existsSync } from 'node:fs';
import { rename, rm } from 'node:fs/promises';
import { lstat, readdir, rename, rm } from 'node:fs/promises';
import { basename, dirname, join } from 'node:path';
import { EXEC_TIMEOUT_MS } from './config/exec-timeout.js';
@@ -188,8 +188,13 @@ export interface GitFailure {
export interface CloneOptions {
/** Pre-validated operand from `parseGitRepositoryUrl`. */
repository: string;
/** Absolute destination directory. Must NOT exist; created by git. */
/** Absolute destination directory. Must NOT exist (created by git), unless `intoEmptyDir` is set. */
destination: string;
/**
* Clone straight into `destination` when it already exists as an EMPTY directory (the folder a user
* picked), instead of refusing it. A folder with anything in it, or a symlink, is still refused.
*/
intoEmptyDir?: boolean;
/** Optional branch or tag (`--branch <ref> --single-branch`). */
ref?: string;
/** `--depth 1`: history-less but much faster on large repos. */
@@ -847,6 +852,25 @@ export async function probeGitRemote(
};
}
/** True for a real (not symlinked) directory with nothing in it. Any read error counts as not empty. */
async function isEmptyDirectory(path: string): Promise<boolean> {
try {
const st = await lstat(path);
return st.isDirectory() && (await readdir(path)).length === 0;
} catch {
return false;
}
}
/** Remove everything inside `path`, keeping `path` itself. Best effort. */
async function emptyDirectory(path: string): Promise<void> {
try {
for (const entry of await readdir(path)) await rm(join(path, entry), { recursive: true, force: true });
} catch {
/* nothing to clean, or it vanished */
}
}
/**
* Clone `repository` into `destination`.
*
@@ -872,10 +896,19 @@ export async function cloneRepository(opts: CloneOptions): Promise<CloneResult>
};
}
if (existsSync(opts.destination)) {
return {
ok: false,
failure: { code: 'DESTINATION_EXISTS', message: 'The destination directory already exists.', stderr: '' },
};
if (!opts.intoEmptyDir || !(await isEmptyDirectory(opts.destination))) {
return {
ok: false,
failure: { code: 'DESTINATION_EXISTS', message: 'The destination directory already exists.', stderr: '' },
};
}
// The folder is the user's own, so there is no temporary sibling to rename into place: git fills it.
const direct = await runGit(buildCloneArgs(opts), opts.timeoutMs ?? GIT_CLONE_TIMEOUT_MS, MAX_STDERR_BYTES);
if (direct.code === 0 && !direct.spawnError) return { ok: true, stderr: sanitizeGitOutput(direct.stderr) };
// It was empty when the clone began, so whatever is in it now is this attempt's partial tree. The
// folder itself stays.
await emptyDirectory(opts.destination);
return { ok: false, failure: classifyGitFailure(direct.stderr, direct.timedOut, direct.spawnError) };
}
// Sibling of the destination (same filesystem), so the rename is atomic.
+3 -3
View File
@@ -3658,15 +3658,15 @@
</div>
<div class="form-row" id="cloneCustomPathToggleRow">
<label class="checkbox-row"><input type="checkbox" id="cloneCustomPathToggle" onchange="app.toggleCloneCustomPath()"> 📁 Clone into a custom folder</label>
<span class="form-hint">By default the clone goes under ~/codeman-cases. Choose another folder and it is cloned there instead; it is listed like any other case, and deleting the case only unlinks it (your files stay).</span>
<span class="form-hint">By default the clone goes under ~/codeman-cases. Choose a folder and the repository is cloned into that folder itself (it must be empty); it is listed like any other case, and deleting the case only unlinks it (your files stay).</span>
</div>
<div class="form-row" id="cloneCustomPathRow" style="display:none">
<label>Parent Folder</label>
<label>Folder</label>
<div class="path-input-group">
<input type="text" id="cloneCasePath" placeholder="~/projects" autocomplete="off" autocapitalize="off" autocorrect="off" spellcheck="false" oninput="app.updateCloneCasePathPreview()">
<button type="button" class="btn path-input-browse" onclick="app.openCloneCasePathPicker()">Browse&hellip;</button>
</div>
<span class="form-hint" id="cloneCasePathPreview">Pick the folder the clone should be created inside.</span>
<span class="form-hint" id="cloneCasePathPreview">Pick the empty folder to clone into.</span>
</div>
<div class="form-row">
<label>Branch or Tag (optional)</label>
+5 -10
View File
@@ -4142,26 +4142,24 @@ Object.assign(CodemanApp.prototype, {
const hint = document.getElementById('cloneCaseNameHint');
if (hint) {
hint.textContent = custom.checked
? 'Filled in from the URL. Cloned into a folder of this name inside the parent folder below.'
? 'Filled in from the URL. Names the case only: the repository is cloned into the folder chosen below.'
: 'Filled in from the URL. Cloned into ~/codeman-cases/<name>, so deleting the case deletes this working tree.';
}
this.updateCloneCasePathPreview();
},
/** The folder the clone would be created in: the parent field plus the case name. */
/** The folder the repository is cloned into: exactly what the field holds (no case name is added). */
_cloneTargetPath() {
const rawParent = (document.getElementById('cloneCasePath')?.value || '').trim();
const name = (document.getElementById('cloneCaseName')?.value || '').trim();
if (!rawParent || !name) return '';
const raw = (document.getElementById('cloneCasePath')?.value || '').trim();
// Trailing slashes off, but `/` stays the root rather than becoming an empty path.
return `${rawParent.replace(/\/+$/, '')}/${name}`;
return raw.length > 1 ? raw.replace(/\/+$/, '') || '/' : raw;
},
updateCloneCasePathPreview() {
const hint = document.getElementById('cloneCasePathPreview');
if (!hint) return;
const target = this._cloneTargetPath();
hint.textContent = target ? `Will clone into: ${target}` : 'Pick the folder the clone should be created inside.';
hint.textContent = target ? `Will clone into: ${target}` : 'Pick the empty folder to clone into.';
},
openCloneCasePathPicker() {
@@ -4214,14 +4212,12 @@ Object.assign(CodemanApp.prototype, {
onCloneNameEdited() {
// Once the user types a name, autofill stops fighting them.
this._cloneNameEdited = !!document.getElementById('cloneCaseName')?.value.trim();
this.updateCloneCasePathPreview();
},
onCloneUrlInput() {
const url = document.getElementById('cloneRepoUrl')?.value.trim() || '';
const nameInput = document.getElementById('cloneCaseName');
if (nameInput && !this._cloneNameEdited) nameInput.value = this._repoNameFromUrl(url);
this.updateCloneCasePathPreview();
clearTimeout(this._clonePreflightTimer);
this._clonePreflightAbort?.abort();
this._clonePreflightAbort = null;
@@ -4277,7 +4273,6 @@ Object.assign(CodemanApp.prototype, {
// the case name is validated against), but never over a name the user typed.
const nameInput = document.getElementById('cloneCaseName');
if (nameInput && !this._cloneNameEdited && parse.suggestedName) nameInput.value = parse.suggestedName;
this.updateCloneCasePathPreview();
const where = parse.owner ? `${parse.provider} ${parse.owner}/${parse.repo}` : `${parse.provider} ${parse.repo}`;
if (data.gitAvailable === false) {
+8 -11
View File
@@ -714,6 +714,7 @@ export function registerCaseRoutes(
const casesDir = resolveCasesDir(user);
let casePath: string | null;
let intoEmptyDir = false;
if (customPath !== undefined) {
const prepared = await prepareNewCasePath(customPath, {
home: homedir(),
@@ -738,18 +739,12 @@ export function registerCaseRoutes(
: prepared.code === 'EXISTS'
? ApiErrorCode.ALREADY_EXISTS
: ApiErrorCode.INVALID_INPUT,
prepared.reason
);
}
// git clones into a fresh folder it creates itself; an empty existing one is refused rather than
// removed and re-made (the removal could race another writer).
if (prepared.existedEmpty) {
reply.code(409);
return createErrorResponse(
ApiErrorCode.ALREADY_EXISTS,
`${prepared.path} already exists. Pick a parent folder where the case folder does not exist yet.`
prepared.code === 'EXISTS'
? `${customPath} already has files in it. Pick an empty folder, or a folder that does not exist yet.`
: prepared.reason
);
}
intoEmptyDir = prepared.existedEmpty;
casePath = prepared.path;
// The same folder listed as a local case in the cases dir would make deleting it remove files.
if (existsSync(join(casesDir, name))) {
@@ -776,7 +771,8 @@ export function registerCaseRoutes(
return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, `That folder is already the case "${linkedAs}"`);
}
if (
existsSync(casePath) ||
// A custom folder was judged by prepareNewCasePath (an existing empty one is fine).
(customPath === undefined && existsSync(casePath)) ||
linkedCases[name] ||
dockerCases.some((item) => item.name === name) ||
remoteCases.some((item) => item.name === name)
@@ -797,6 +793,7 @@ export function registerCaseRoutes(
const clone = await cloneRepository({
repository: parsed.repository,
destination: casePath,
...(intoEmptyDir ? { intoEmptyDir: true } : {}),
withoutCredentialHelpers: cloneWithoutCredentialHelpers(req),
...(ref ? { ref } : {}),
...(shallow ? { shallow: true } : {}),
+35 -10
View File
@@ -1,14 +1,24 @@
/**
* @fileoverview POST /api/cases/clone with a `path`: clone into a custom folder and register it as a
* linked case. Real filesystem under test/setup.ts's temp HOME and a REAL `git` cloning a local bare
* repo (the path policy itself is in test/case-path.test.ts). Port: N/A (app.inject()).
* @fileoverview POST /api/cases/clone with a `path`: clone INTO that folder (an empty or not yet existing one,
* with no subfolder added) and register it as a linked case. Real filesystem under test/setup.ts's temp
* HOME and a REAL `git` cloning a local bare repo (the path policy itself is in test/case-path.test.ts).
* Port: N/A (app.inject()).
*
* Like case-custom-path-routes.test.ts this suite rewrites the linked-cases registry, so it refuses to
* start outside the sandboxed HOME (a raw `npx vitest` would reach the real ~/.codeman).
*/
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
import { execFileSync } from 'node:child_process';
import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
import {
existsSync,
mkdirSync,
mkdtempSync,
readdirSync,
readFileSync,
realpathSync,
rmSync,
writeFileSync,
} from 'node:fs';
import { homedir, tmpdir } from 'node:os';
import { join } from 'node:path';
import { createRouteTestHarness } from './_route-test-utils.js';
@@ -103,18 +113,33 @@ describe.skipIf(!isGitAvailable())('POST /api/cases/clone with a custom folder',
writeFileSync(join(target, 'mine.txt'), 'keep me');
const res = await clone(app, { name: 'busy', repository: origin, path: target });
expect(res.statusCode).toBe(409);
expect(res.json().error).toMatch(/already has files/);
expect(readFileSync(join(target, 'mine.txt'), 'utf8')).toBe('keep me');
expect(linked()).toEqual({});
});
it('refuses an existing EMPTY folder rather than removing and re-making it', async () => {
it('clones straight INTO the chosen empty folder, with no subfolder named after the case', async () => {
const { app } = await createRouteTestHarness(registerCaseRoutes);
const target = join(workDir, 'empty');
const target = join(workDir, 'picked');
mkdirSync(target);
const res = await clone(app, { name: 'empty', repository: origin, path: target });
expect(res.statusCode).toBe(409);
expect(res.json().error).toMatch(/already exists/);
expect(existsSync(join(target, '.git'))).toBe(false);
const res = await clone(app, { name: 'a-different-name', repository: origin, path: target });
expect(res.statusCode).toBe(200);
expect(res.json().data.case).toEqual({ name: 'a-different-name', path: target });
expect(existsSync(join(target, 'README.md'))).toBe(true);
expect(existsSync(join(target, '.git'))).toBe(true);
expect(existsSync(join(target, 'a-different-name'))).toBe(false);
expect(linked()).toEqual({ 'a-different-name': target });
});
it('leaves the empty folder in place, and empty, when the clone fails', async () => {
const { app } = await createRouteTestHarness(registerCaseRoutes);
const target = join(workDir, 'stays');
mkdirSync(target);
const res = await clone(app, { name: 'stays', repository: join(root, 'no-such-repo.git'), path: target });
expect(res.json().success).toBe(false);
expect(existsSync(target)).toBe(true);
expect(readdirSync(target)).toEqual([]);
expect(linked()).toEqual({});
});
it('refuses a blocked tree such as ~/.ssh', async () => {