diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 479d279b5..262192a57 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -474,7 +474,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) 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. +**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` 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. @@ -485,7 +485,7 @@ Tests: `test/file-preview-markdown.test.ts` (jsdom-in-vm, pins every rule above) - **Failure leaves nothing behind.** The destination is removed only when it did not exist before the attempt, and a pre-existing directory is refused rather than cloned into, so a failed clone never squats on a case name and never touches an existing tree. - ⚠️ **Error detail comes from the LAST diagnostic line, not the first.** `git clone` opens with `Cloning into ''…`, so a first-line pick reported the destination path as the reason a bad branch failed (observed against a real remote). `NOT_FOUND` wording must also say "or private": GitHub answers "Repository not found" for a private repo and a typo alike when unauthenticated. - **Multi-user**: NOT admin-gated, unlike `/api/cases/link` — it writes only inside the caller's own `resolveCasesDir`. The exception is a `local`-transport source (an absolute path or `file://`), which is admin-only there because per-user spaces live inside one `$HOME` and a local clone would read straight through that boundary. -- **UI** (`case-clone` tab in the Add Case modal): debounced preflight paints a verdict under the field, fills the case name from the parsed repo (until the user types their own), and turns the branch/tag field into a datalist of the remote's real refs. The **Brain** picker sets the toolbar run mode on success (gated by `isCliAvailable()`, like `#runModeMenu`), so Run already points at the chosen CLI; starting a session stays opt-in. The tab hides itself when the server reports no `git` (injected via `window.__codemanCliAvailable`). +- **UI** (`case-clone` tab in the Add Case modal): the **Clone into a custom folder** field is a PARENT folder, like Create New's: the tab sends `/` as `path` (`_cloneTargetPath()`), so the API's `path` stays the exact target; debounced preflight paints a verdict under the field, fills the case name from the parsed repo (until the user types their own), and turns the branch/tag field into a datalist of the remote's real refs. The **Brain** picker sets the toolbar run mode on success (gated by `isCliAvailable()`, like `#runModeMenu`), so Run already points at the chosen CLI; starting a session stays opt-in. The tab hides itself when the server reports no `git` (injected via `window.__codemanCliAvailable`). Tests: `test/git-clone.test.ts` (pure half exhaustively, plus REAL git against a REAL local bare repo for clone/ref/timeout/cleanup), `test/routes/case-clone-routes.test.ts` (deliberately **unmocked fs**, real clone through the endpoint). diff --git a/docs/wiki/Core-Concepts.md b/docs/wiki/Core-Concepts.md index 1b2428a08..6f2cd0a0b 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 straight into a folder you choose, which must be empty or not exist yet) 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, with **Clone into a custom folder**, into `/`, which must be empty or not exist yet) 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/web/public/index.html b/src/web/public/index.html index 039d55f7f..225913d4c 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -3791,15 +3791,15 @@

Clone Repo

- By default the clone goes under ~/codeman-cases. Choose a folder and the repository is cloned into that folder itself (an empty folder, or one that does not exist yet); 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 instead and the repository is cloned into a subfolder of it named after the case, the same way it goes under ~/codeman-cases; 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 fc0d25828..fd57b80cf 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -4264,30 +4264,34 @@ Object.assign(CodemanApp.prototype, { const hint = document.getElementById('cloneCaseNameHint'); if (hint) { hint.textContent = custom.checked - ? 'Filled in from the URL. Names the case only: the repository is cloned into the folder chosen below.' + ? 'Filled in from the URL. Names the case and the subfolder of the chosen folder the repository is cloned into.' : 'Filled in from the URL. Cloned into ~/codeman-cases/, so deleting the case deletes this working tree.'; } this.updateCloneCasePathPreview(); }, - /** The folder the repository is cloned into: exactly what the field holds (no case name is added). */ + /** The folder the repository is cloned into: the parent field plus the case name, as Create New does. */ _cloneTargetPath() { - const raw = (document.getElementById('cloneCasePath')?.value || '').trim(); + const rawParent = (document.getElementById('cloneCasePath')?.value || '').trim(); + const name = (document.getElementById('cloneCaseName')?.value || '').trim(); + if (!rawParent || !name) return ''; // Trailing slashes off, but `/` stays the root rather than becoming an empty path. - return raw.length > 1 ? raw.replace(/\/+$/, '') || '/' : raw; + return `${rawParent.replace(/\/+$/, '')}/${name}`; }, updateCloneCasePathPreview() { const hint = document.getElementById('cloneCasePathPreview'); if (!hint) return; const target = this._cloneTargetPath(); - hint.textContent = target ? `Will clone into: ${target}` : 'Pick an empty folder, or one that does not exist yet, to clone into.'; + hint.textContent = target + ? `Will clone into: ${target}` + : 'Pick the folder the repository should be cloned inside. It goes into a subfolder named after the case.'; }, openCloneCasePathPicker() { const input = document.getElementById('cloneCasePath'); PathPicker.open({ - title: 'Choose the folder to clone into', + title: 'Choose the folder to clone inside', initialPath: input.value.trim(), directoriesOnly: true, onSelect: (path) => { @@ -4334,12 +4338,14 @@ 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; @@ -4395,6 +4401,7 @@ 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) { @@ -4449,7 +4456,7 @@ Object.assign(CodemanApp.prototype, { const customFolder = !!document.getElementById('cloneCustomPathToggle')?.checked; if (customFolder && !(document.getElementById('cloneCasePath')?.value || '').trim()) { - this.showToast('Choose the folder to clone into', 'error'); + this.showToast('Choose the folder to clone inside', 'error'); return; } diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 0cfc5f5d9..7d295bb35 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -748,7 +748,7 @@ export function registerCaseRoutes( ? ApiErrorCode.ALREADY_EXISTS : ApiErrorCode.INVALID_INPUT, prepared.code === 'EXISTS' - ? `${customPath} already has files in it. Pick an empty folder, or a folder that does not exist yet.` + ? `${customPath} already has files in it. The clone target must be empty or not exist yet.` : prepared.reason ); } diff --git a/test/clone-custom-folder-ui.test.ts b/test/clone-custom-folder-ui.test.ts new file mode 100644 index 000000000..4e5e8a245 --- /dev/null +++ b/test/clone-custom-folder-ui.test.ts @@ -0,0 +1,101 @@ +/** + * @fileoverview Frontend test for Clone Repo's custom folder (session-ui.js `cloneCase()` / + * `_cloneTargetPath()`): the chosen folder is the PARENT, so the clone request carries + * `/` exactly as Create New does, and without the checkbox no `path` is sent + * (the server then clones to `~/codeman-cases/`). Runs against the real index.html markup. + * + * Port: none. + */ +import { readFileSync } from 'node:fs'; +import { JSDOM } from 'jsdom'; +import { describe, expect, it, vi } from 'vitest'; + +const INDEX_HTML = readFileSync(new URL('../src/web/public/index.html', import.meta.url), 'utf-8'); +const CONSTANTS_JS = readFileSync(new URL('../src/web/public/constants.js', import.meta.url), 'utf-8'); +const SESSION_UI_JS = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf-8'); + +function bootClone() { + const dom = new JSDOM(INDEX_HTML, { url: 'http://localhost/', runScripts: 'outside-only' }); + const win = dom.window as unknown as Window & typeof globalThis & { CodemanApp: new () => any }; + const run = (src: string) => (win as unknown as { eval: (s: string) => void }).eval(src); + run('window.CodemanApp = function CodemanApp() {};'); + run(CONSTANTS_JS); + run(SESSION_UI_JS); + const app = new win.CodemanApp(); + app.showToast = () => {}; + app._setCloneStatus = () => {}; + app.closeCreateCaseModal = () => {}; + app.loadQuickStartCases = async () => {}; + app.saveLastUsedCase = async () => {}; + const fetchMock = vi.fn(async () => ({ json: async () => ({ success: true, data: {} }) })); + (win as unknown as { fetch: unknown }).fetch = fetchMock; + const doc = win.document; + const set = (id: string, value: string) => ((doc.getElementById(id) as HTMLInputElement).value = value); + set('cloneRepoUrl', 'https://github.com/opticon454/bifrost'); + set('cloneCaseName', 'bifrost'); + return { app, doc, set, fetchMock, win }; +} + +async function requestBody(fetchMock: ReturnType) { + const [url, init] = fetchMock.mock.calls[0] as unknown as [string, { body: string }]; + return { url, body: JSON.parse(init.body) }; +} + +describe('Clone Repo custom folder', () => { + it('sends / as the path and previews it', async () => { + const { app, doc, set, fetchMock } = bootClone(); + (doc.getElementById('cloneCustomPathToggle') as HTMLInputElement).checked = true; + set('cloneCasePath', '/mnt/user/Scripts/GitHub/opticon454/'); + app.updateCloneCasePathPreview(); + expect(doc.getElementById('cloneCasePathPreview')?.textContent).toBe( + 'Will clone into: /mnt/user/Scripts/GitHub/opticon454/bifrost' + ); + await app.cloneCase(); + const { url, body } = await requestBody(fetchMock); + expect(url).toBe('/api/cases/clone'); + expect(body.path).toBe('/mnt/user/Scripts/GitHub/opticon454/bifrost'); + expect(body.name).toBe('bifrost'); + }); + + it('follows the case name when it is edited', () => { + const { app, doc, set } = bootClone(); + set('cloneCasePath', '~/projects'); + set('cloneCaseName', 'other'); + app.onCloneNameEdited(); + expect(doc.getElementById('cloneCasePathPreview')?.textContent).toBe('Will clone into: ~/projects/other'); + }); + + it('sends no path without the checkbox, even when the folder field still holds one', async () => { + const { app, set, fetchMock } = bootClone(); + set('cloneCasePath', '/mnt/user/Scripts/GitHub/opticon454'); + await app.cloneCase(); + const { body } = await requestBody(fetchMock); + expect(body.path).toBeUndefined(); + }); + + it('refreshes the preview when the preflight replaces the case name', () => { + const { app, doc, set } = bootClone(); + set('cloneRepoUrl', 'https://github.com/opticon454/bifrost?tab=readme-ov-file'); + set('cloneCaseName', 'bifrost-tab-readme-ov-file'); + set('cloneCasePath', '/mnt/projects'); + app.updateCloneCasePathPreview(); + expect(doc.getElementById('cloneCasePathPreview')?.textContent).toBe( + 'Will clone into: /mnt/projects/bifrost-tab-readme-ov-file' + ); + app._applyClonePreflight({ gitAvailable: true, parse: { cloneable: true, suggestedName: 'bifrost' } }, ''); + expect((doc.getElementById('cloneCaseName') as HTMLInputElement).value).toBe('bifrost'); + expect(doc.getElementById('cloneCasePathPreview')?.textContent).toBe('Will clone into: /mnt/projects/bifrost'); + }); + + it('previews the name taken from the URL as it is typed', () => { + const { app, doc, set, win } = bootClone(); + app.isCliAvailable = () => true; // lives in settings-ui.js, which this test does not load + set('cloneCaseName', ''); + set('cloneCasePath', '/mnt/projects'); + set('cloneRepoUrl', 'https://github.com/opticon454/bifrost'); + app.onCloneUrlInput(); + win.clearTimeout(app._clonePreflightTimer); + expect((doc.getElementById('cloneCaseName') as HTMLInputElement).value).toBe('bifrost'); + expect(doc.getElementById('cloneCasePathPreview')?.textContent).toBe('Will clone into: /mnt/projects/bifrost'); + }); +});