diff --git a/CLAUDE.md b/CLAUDE.md index 56ba4e2d..343f6cd6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 `/.claude/worktrees/` on `worktree-`, 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) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 63f047b0..4b7a7ec5 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -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 ` makes git run an arbitrary command as its transport, and ANY `::` dispatches to a `git-remote-` 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. diff --git a/docs/wiki/Core-Concepts.md b/docs/wiki/Core-Concepts.md index 85fb0416..fa849ba4 100644 --- a/docs/wiki/Core-Concepts.md +++ b/docs/wiki/Core-Concepts.md @@ -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/` 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/` (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/` (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 diff --git a/src/git-clone.ts b/src/git-clone.ts index ef3380e8..249e4234 100644 --- a/src/git-clone.ts +++ b/src/git-clone.ts @@ -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 --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 { + 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 { + 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 }; } 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. diff --git a/src/web/public/index.html b/src/web/public/index.html index 195e5900..d88d2ec7 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -3658,15 +3658,15 @@
- 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). + 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).
diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index ea462d4b..c1455d00 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -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/, 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) { diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 51c09a39..cb7f9389 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -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 } : {}), diff --git a/test/routes/case-clone-custom-folder.test.ts b/test/routes/case-clone-custom-folder.test.ts index 0c700387..a5c1ac49 100644 --- a/test/routes/case-clone-custom-folder.test.ts +++ b/test/routes/case-clone-custom-folder.test.ts @@ -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 () => {