Skip to content

feat(cases): clone a repository into a custom folder - #744

Open
opticon454 wants to merge 5 commits into
Ark0N:masterfrom
opticon454:feature/clone-custom-folder
Open

opticon454 wants to merge 5 commits into
Ark0N:masterfrom
opticon454:feature/clone-custom-folder

Conversation

@opticon454

@opticon454 opticon454 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Context

Add Case has a Clone Repo tab, but it can only clone into ~/codeman-cases/<name>. Create New already lets you pick a folder instead (Create in a custom folder, POST /api/cases with a path); Clone Repo had no equivalent, so anyone who keeps projects somewhere else (a work drive, ~/projects, a mount) had to clone by hand and then use Link Existing.

This adds the same option to Clone Repo: Clone into a custom folder, with a Folder field and a Browse button. The repository is cloned into the folder you pick (no subfolder is added, the case name does not affect the path) and is registered as a linked case, so it lists, resolves and deletes like any other linked case.

What changed

  • POST /api/cases/clone takes an optional path: the folder to clone into, exactly as given. Without it nothing changes.
  • The path is judged by prepareNewCasePath(), the same policy Create in a custom folder uses: absolute or ~, no traversal or shell characters, not a system directory, the home folder itself, Codeman's data folder, a credentials tree (~/.ssh, ~/.aws, ...), or the cases folder or one of its direct children; the parent must already exist; symlinks are resolved and judged again.
  • Admin only in multi-user mode, like Create in a custom folder and Link Existing: it writes an absolute path into the shared, ownerless linked-cases registry. A non-admin gets a 403 before anything runs, and the checkbox is hidden for them.
  • Never clones at or inside a Docker case workspace (400, 503 when the Docker case list cannot be read), as git status, fetch and worktrees do: git would run the new repository's hooks and filters on this machine.
  • The folder must be empty or not exist yet (its parent must exist). An existing empty folder is cloned into directly (CloneOptions.intoEmptyDir): no temporary sibling and no rename, so a mount point works, and a failed or timed-out attempt empties it again but never removes the folder. A symlink, or a folder with anything in it, is refused with a message saying so.
  • Everything else about the clone is unchanged: the URL rules, anonymous or helper credentials, the timeout, the temporary sibling directory that is renamed into place, scaffolding that never overwrites what the repository ships, and the warnings for repo-supplied hooks.
  • The registry entry is written after git finishes, re-reading the file first so a case linked while the clone ran is not dropped. If that write fails, the cloned folder is kept (the user waited for it) and the error says where it is and to use Link Existing.
  • Deleting such a case unlinks it and leaves the files, as for any linked case.

The UI mirrors Create New: a checkbox, a Folder field with Browse, and a live "Will clone into: ..." preview. The case name only names the case.

Verification

  • New test/routes/case-clone-custom-folder.test.ts (22 tests, real git against a local bare repo, real filesystem under the sandboxed HOME): clone into a new folder with scaffold, hooks and registry entry; clone straight into an existing empty folder with no <case name> subfolder, even when the name differs; the empty folder stays, and stays empty, when the clone fails; ~ expansion; missing parent (404, nothing created); non-empty target (409, files untouched); ~/.ssh refused; name already linked; failed clone into a new folder leaves nothing and no registry entry; no path still clones under the cases folder unlinked; two clones into one empty folder at once (exactly one wins, its files survive); a name linked while the clone ran (409, folder kept and named); a direct clone whose git-slot wait times out leaves what was written into the folder meanwhile; a folder at or inside a Docker case workspace is refused and nothing is created; an unreadable Docker case list fails closed (503); a refused folder named plain, openssl, rustls or does not exist keeps what was written during the git-slot wait and reports DESTINATION_EXISTS; the Docker refusal on the default (no path) branch; multi-user non-admin 403 with nothing cloned, admin allowed.
  • npm run typecheck, lint, format:check, check:frontend-syntax clean.
  • Full npm test on the latest commit: 667 files, 13,526 tests passed, 12 skipped, 0 failed.
  • Docs: docs/wiki/Core-Concepts.md, the Clone Repo paragraph in docs/architecture-invariants.md and CLAUDE.md.

Commits

  • 0bd15dd feat(cases): clone a repository into a custom folder
  • 68bd7e4 fix(cases): clone into the chosen folder itself, not a subfolder of it
  • 086249a fix(cases): never delete another clone's files after a failed direct clone
  • b549a66 fix(cases): skip the cleanup when git never ran, and refuse Docker workspaces
  • 41186f5 fix(cases): never let a path decide whether a refused clone empties the folder

Clone Repo could only clone under ~/codeman-cases. POST /api/cases/clone now takes an optional path (parent folder plus case name) and the Clone tab gets a Clone into a custom folder option with a Browse button, mirroring Create in a custom folder. The path is judged by prepareNewCasePath, the clone is registered as a linked case, the option is admin-only in multi-user mode, and an existing target folder is refused.
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.
@opticon454

Copy link
Copy Markdown
Contributor Author

Pushed 68bd7e4. The folder you pick is now the clone destination itself: the repository is cloned into it directly rather than into a <case name> subfolder of it. An existing empty folder is cloned into in place (no temporary sibling or rename, so a mount point works) and is kept, and emptied again, if the clone fails; a folder with files, or a symlink, is refused. The case name only names the case. Full npm test: 13,515 passed, 0 failed. PR body updated.

@Ark0N

Ark0N commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Thanks @opticon454 for this! It lets Clone Repo clone straight into a folder you pick (empty or not yet created) and registers it as a linked case, with the same path policy and admin gate as Create in a custom folder. Reusing prepareNewCasePath(), re-reading the registry before the write, and moving the docs along with the code are all exactly right.

One thing needs fixing before merge.

1. A failed clone into an existing empty folder deletes whatever is in it, including another attempt's finished clone (src/git-clone.ts:910)

emptyDirectory() runs after every failed direct clone, on the premise that anything in the folder is this attempt's partial tree. That premise breaks whenever git wrote nothing: git refused because the folder was no longer empty (DESTINATION_EXISTS, line 636), or git never ran (slot timeout, full queue). The emptiness check happens before runGit waits for a git slot, so that gap can be long. I ran two cloneRepository({ intoEmptyDir: true }) calls together into one empty folder against a local bare repo, 20 times:

  • with CODEMAN_MAX_GIT_OPERATIONS=1 (the same as a request queued behind busy slots): 20 of 20 runs, the first clone returns ok, the second one's git refuses with "destination path already exists and is not an empty directory", and its cleanup deletes the first clone completely. Through the route, the first request has already answered 200 and written linked-cases.json pointing at a folder that is now empty.
  • with the default pool of 2: 19 of 20 runs, both fail and the folder ends empty.

This is the regression the cloneRepository doc comment says the temp-sibling design prevents ("each attempt only ever creates and removes its own directory"), and the CLAUDE.md clone rule says to remove the destination only if this attempt created it. The modal's disabled button does not cover it: two tabs, an API caller, or a retry after a proxy timeout while the first request is still queued.

Asks:

  • (a) Skip emptyDirectory() when git wrote nothing: when the failure classifies as DESTINATION_EXISTS, or on a spawnError.
  • (b) Make clones into one destination mutually exclusive in-process: an in-flight Set<string> keyed on the destination in cloneRepository, taken before isEmptyDirectory() and released in a finally, so a second caller gets DESTINATION_EXISTS before spawning. Once two gits have written into one folder their files cannot be told apart, so (a) alone is not enough.
  • (c) A regression test: Promise.all two direct clones into one empty folder and assert that exactly one is ok and that its README.md and .git are still there. The existing "leaves the empty folder in place, and empty" test clones a repository that does not exist, so git never writes anything and that test cannot catch this.

Smaller things, fine to fold into the same push:

  • src/web/routes/case-routes.ts:862: a name linked while the clone ran answers 500 OPERATION_FAILED, while createCaseInCustomFolder answers the same conflict 409 ALREADY_EXISTS. Please match that (the "kept the folder, use Link Existing" message is good as it is), and add a test for this branch that checks the folder is kept and named in the error.
  • src/web/schemas.ts:789: the comment still says "(parent + case name)". Since 68bd7e4 the path is the destination folder itself.
  • docs/api-reference.md: the clone route has no section yet (that predates this PR), so path is undocumented. Please add a short paragraph next to the POST /api/cases path one (around line 1305) with its statuses: 400, 403, 404, 409, 422, and the 500 that keeps the folder. The wiki row in docs/wiki/Core-Concepts.md says "straight into an empty folder you choose", but a folder that does not exist yet works too.
  • src/git-clone.ts:910: when a clone times out because a network mount stopped answering, the unbounded readdir/rm in emptyDirectory() holds a libuv worker until the mount comes back (the fix(cases): bound path probes for linked workspaces and session creation, so an unreachable mount cannot freeze the server #516 rule in CLAUDE.md). Since item 1 touches this line anyway: run probePathKind(dest, { pastCap: true }) first and skip the cleanup on unknown.

Once item 1 is in with its test, this is ready to merge. Thanks again for picking this up.

…clone

A direct clone into an empty folder emptied the folder after any failure, which
deleted a concurrent attempt's finished clone when git had written nothing.
Clones into one folder are now mutually exclusive in-process, the cleanup is
skipped when git refused the folder, never ran, or the mount is unresponsive,
and a name linked while the clone ran is a 409 that keeps the folder.
Documents path for POST /api/cases/clone.
@opticon454

Copy link
Copy Markdown
Contributor Author

Pushed 086249a, thanks for the careful repro.

  • (a) The cleanup now runs only when git could have written something: it is skipped on DESTINATION_EXISTS and on a spawnError, and it first runs probePathKind(dest, { pastCap: true }) and skips on anything but directory, so an unresponsive mount does not hold a libuv worker.
  • (b) cloneRepository keeps an in-flight Set keyed on the destination, taken before isEmptyDirectory() and released in a finally. A second caller gets DESTINATION_EXISTS before it spawns.
  • (c) New test: Promise.all of two direct clones into one empty folder asserts exactly one ok, the loser is DESTINATION_EXISTS, and README.md and .git are still there. I checked it fails (the folder ends empty) with (a) and (b) removed.
  • A name linked while the clone ran is now 409 ALREADY_EXISTS, with a test that checks the folder is kept and named in the error.
  • The schema comment, the wiki row (empty or not yet created) and a docs/api-reference.md paragraph for POST /api/cases/clone path (400, 403, 404, 409, 422, and the 500 that keeps the folder) are updated.

Full npm test: 13,517 passed, 0 failed. PR body updated.

@Ark0N

Ark0N commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Thanks @opticon454 for the quick turnaround on the last round. This PR adds Clone into a custom folder to Clone Repo: the repository is cloned into a folder you pick (empty or not yet created) and registered as a linked case, under the same path policy and admin gate as Create in a custom folder. The in-flight Set, the DESTINATION_EXISTS skip, the 409 for a name linked mid-clone and the docs are all in and work as described, and typecheck, lint, format and the case and clone test files are green here.

Two things still need fixing before merge.

1. A direct clone whose git-slot wait times out still empties the folder (src/git-clone.ts:924)

This is the "git never ran (slot timeout)" half of item 1 from my last comment, and the gap is in my ask, not in your code: I assumed a slot timeout comes back with a spawnError. It does not. runGit returns { code: null, timedOut: true } with no spawnError when the queue wait runs out (src/git-clone.ts:731), so the guard on line 924 passes and emptyDirectory() runs on a folder git never touched. The wait spends the clone's own deadline (5 minutes by default) and the Set only keeps out other direct clones, so anything else written into the folder in that window is deleted: files the user copies in, or a Create New with the same custom folder (POST /api/cases with path), which then keeps its linked-cases entry while its scaffold is wiped. The comment on lines 921-922 and the new CLAUDE.md line say this case is skipped.

I reproduced it: two clones of ssh://localhost/x.git with GIT_SSH_COMMAND='sleep 6; true' held both git slots (no network needed), a direct clone with timeoutMs: 1500 queued behind them (getActiveGitOperationCount() 2, getQueuedGitOperationCount() 1), a file and a src/ folder were written into the target during the wait, and the call returned TIMEOUT with the folder empty.

Asks:

  • Have runGit report whether git was spawned (for example started: boolean on GitRun, false for queue-full and timed-out) and require it before the cleanup, so the condition states the real premise.
  • A regression test shaped like the repro above, asserting that the file written during the wait is still there.

2. Refuse a path at or inside a Docker case workspace (src/web/routes/case-routes.ts:719)

I missed this in the first round. Codeman never runs git in a folder a container can write to: Git status, diff, fetch and worktrees all refuse a repository at or inside a Docker case's hostWorkspacePath, because git runs that folder's hooks and filters on the host (defaultDockerWorkspaces() in src/web/routes/git-status-routes.ts, which is "checked by every path that could run git"). git clone is such a path: it runs .git/hooks/post-checkout of the new repository once checkout finishes (checked here with git 2.43), and it applies filters configured in .git/config. A container with the workspace bind-mounted can write both during the fetch, before checkout, and its code then runs on this machine as Codeman's user. Nothing in the route or in prepareNewCasePath() stops a custom folder inside a Docker workspace today, and cloning into one so a sandboxed agent can work on it is a natural use of this feature. The temporary sibling for a folder that does not exist yet sits in the same parent, so that branch is exposed the same way.

Asks:

  • Before cloneRepository, refuse with 400 INVALID_INPUT a resolved destination at or inside any realpath-resolved Docker workspace. defaultDockerWorkspaces() already fails closed on an unreadable docker-cases.json, as the fetch paths need, and the worktree refusal text at src/git-worktrees.ts:487 is a good model for the message.
  • A route test that registers a Docker case with a workspace and asserts that a clone into a folder inside it is refused and creates nothing.

The default <cases dir>/<name> branch can only land inside a Docker workspace when one is set to the cases folder or above it. That predates this PR, so covering it with the same check is welcome but not required.

Optional, fine to leave as it is: cloneRepository now reaches a user-chosen path with a synchronous existsSync (line 909) and an unbounded lstat/readdir in isEmptyDirectory(). prepareNewCasePath() probed the parent bounded just before, so this is the same residual race other routes accept under the #516 rule in CLAUDE.md.

Once both items are in with their tests, this is ready to merge. Thanks again for sticking with it.

…rkspaces

A direct clone whose git-slot wait timed out came back with no spawnError, so
the cleanup emptied a folder git never touched. GitRun now says whether git
started and the cleanup requires it. Cloning into a folder at or inside a
Docker case workspace is refused, as git status, fetch and worktrees do, since
git would run the new repository's hooks and filters on the host.
@opticon454

Copy link
Copy Markdown
Contributor Author

Pushed b549a66, thanks. Both points were right.

  1. GitRun now carries started (false for queue-full, timed-out and a failed spawn, true once git ran to a close), and the cleanup requires direct.started as well as a non-DESTINATION_EXISTS failure and a directory probe. The comment and the CLAUDE.md line now state the real premise. New test shaped like your repro: two slow ssh clones hold both slots, a direct clone with timeoutMs: 1000 queues behind them, a file and a src/ folder are written into the target during the wait, and the call returns TIMEOUT with both still there. I checked it fails with the old guard.
  2. Before cloneRepository the route resolves every defaultDockerWorkspaces() entry and refuses 400 INVALID_INPUT a destination at or inside one. It fails closed (503) when the Docker case list cannot be read. I applied it to the default <cases dir>/<name> branch too, through the real cases dir. Tests: a clone into the workspace itself and into a folder inside it is refused with nothing created and a sibling folder still clones; an unreadable list gives 503.

Docs: docs/api-reference.md and the CLAUDE.md line mention both. Full npm test: 13,520 passed, 0 failed. PR body updated.

@Ark0N

Ark0N commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Thanks @opticon454 for the third round. This PR lets Clone Repo clone straight into a folder you pick (empty or not yet created) and registers it as a linked case, under the same path policy and admin gate as Create in a custom folder. Both items from my last comment are in and work as described: GitRun.started now gates the cleanup, the slot-timeout test is exactly the right shape, and the Docker refusal (on both branches, failing closed) matches the other git paths. Typecheck, lint, format, frontend syntax and the test suite are green here.

One thing still needs fixing before merge.

1. A failed direct clone still empties a folder git never wrote to when its path contains ssl or tls (src/git-clone.ts:936, cause at src/git-clone.ts:634)

The cleanup is skipped only when failure.code === 'DESTINATION_EXISTS', and that code comes from classifyGitFailure(), which tests stderr with substring patterns in order. git's refusal embeds the destination path (fatal: destination path '/home/u/src/openssl' already exists and is not an empty directory.), and the HOST_UNREACHABLE pattern on line 634 (...|ssl|tls) runs before the DESTINATION_EXISTS one on line 637. So for any folder named after openssl, boringssl, libressl, rustls, mbedtls and so on, the refusal classifies as HOST_UNREACHABLE, started is true, and emptyDirectory() deletes whatever was written into the folder during the git-slot wait (files the user copied in, or a Create New scaffold for the same folder). The user is also told "Could not reach that host". The NOT_FOUND pattern on line 623 does the same for a path that spells not found or does not exist. The ordering predates this PR, but on master it could never decide a deletion, since the default path only removes its own temp sibling.

I reproduced it with your slot-hold harness: two ssh://localhost/x.git clones with GIT_SSH_COMMAND='sleep 2; true' hold both slots, a direct clone with timeoutMs: 15_000 queues behind them (so it does get a slot), and mine.txt is written into the target during the wait. With the folder named plain the result is DESTINATION_EXISTS and the file survives; with it named openssl the result is HOST_UNREACHABLE and the folder ends empty.

Asks:

  • (a) In the direct-clone branch, test the sanitized stderr for already exists and is not an empty directory directly and skip the cleanup on a match, so the deletion no longer depends on the classifier's order.
  • (b) Move the DESTINATION_EXISTS match to the top of classifyGitFailure and tighten ssl|tls (for example \bssl\b|\btls\b|certificate|handshake), which also fixes the message on both clone paths.
  • (c) A regression test shaped like the slot-timeout one, with the target folder named openssl and a timeout long enough that git runs, asserting the file written during the wait survives and the failure is DESTINATION_EXISTS.

Smaller things, fine to fold into the same push:

  • src/web/routes/case-routes.ts:798: the Docker check on the default <cases dir>/<name> branch has no test. One route test with a Docker case whose hostWorkspacePath is the cases folder, asserting a clone without path is refused INVALID_INPUT and creates nothing, would pin it.
  • src/web/routes/case-routes.ts:152: DOCKER_CLONE_REFUSAL landed between LOCAL_CLONE_ADMIN_ONLY and its JSDoc (line 147), so that comment now documents the wrong constant. Please move the new constant below it with its own one-line comment.
  • src/web/routes/case-routes.ts:668: the route JSDoc still says the route writes only inside the caller's own resolveCasesDir and is not admin-gated. One sentence on the path exception.
  • src/web/public/index.html:3661 and :3669, src/web/public/session-ui.js:4162: the copy says the folder must be empty, but a folder that does not exist yet works too (the wiki row already says so). Something like "an empty folder, or one that does not exist yet".

Once item 1 is in with its test, this is ready to merge. Thanks again for sticking with it through three rounds.

…he folder

git's refusal quotes the destination, so a folder named openssl or rustls
classified as HOST_UNREACHABLE and the cleanup emptied it. The cleanup now
tests git's own text, DESTINATION_EXISTS is matched first, and ssl/tls are
matched as words. Also: a test for the Docker refusal on the default branch,
the route JSDoc and constant comment, and the folder copy (empty, or new).
@opticon454

Copy link
Copy Markdown
Contributor Author

Pushed 41186f5, thanks. The repro was exactly right.

  • (a) The direct-clone cleanup now tests git's own text (already exists and is not an empty directory) and skips on a match, as well as on !started and a non-directory probe, so the deletion no longer depends on the classifier.
  • (b) DESTINATION_EXISTS is matched first in classifyGitFailure, and the host pattern is now \bssl\b|\btls\b|certificate|handshake (plus the existing words). A classifier test checks the refusal for openssl, rustls, tls-test and a path spelling repository not found, and that a real SSL, TLS handshake, DNS and not-found failure still classify as before.
  • (c) Regression test shaped like the slot-timeout one, run for folders named plain, openssl, rustls and does not exist, with a 15 s timeout so git runs: the failure is DESTINATION_EXISTS and the file written during the wait survives. I checked it fails (all four) when the cleanup guard is removed.

Smaller items: a route test for the Docker refusal on the default <cases dir>/<name> branch (a Docker case whose workspace is the cases folder), DOCKER_CLONE_REFUSAL moved below LOCAL_CLONE_ADMIN_ONLY with its own comment, the route JSDoc now notes the path exception, and the copy in index.html and session-ui.js says "an empty folder, or one that does not exist yet".

Full npm test: 13,526 passed, 0 failed. PR body updated.

@Ark0N

Ark0N commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Thanks @opticon454 for this, and for staying with it through four rounds. This PR adds Clone into a custom folder to Clone Repo: the repository is cloned straight into a folder you pick (empty or not yet created) and registered as a linked case, under the same path policy and admin gate as Create in a custom folder, and Clone Repo now refuses a destination at or inside a Docker case workspace on both branches.

Every ask from the last round is in and works as described: the cleanup is gated on git's own refusal text and on GitRun.started, DESTINATION_EXISTS is matched first, the openssl/rustls/does not exist regression test is the right shape, and the Docker refusal on the default branch has its test. Typecheck, lint, format, frontend syntax, the case and clone test files and the full npm test are green here, and so is CI.

A few small things are left. None of them needs another push from you; I will apply them on top when merging:

  1. src/git-clone.ts:640: the tightened TLS pattern no longer catches OpenSSL's and LibreSSL's SSL_connect errors. \bssl\b cannot match SSL_connect or SSL_ERROR_SYSCALL, because _ is a word character. So fatal: unable to access '...': OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to github.com:443, a common flaky-network or proxy error (and the usual one on macOS with LibreSSL), now classifies as FAILED instead of HOST_UNREACHABLE. On the default branch the user then reads git failed: Cloning into '/home/u/codeman-cases/.x.cloning-ab12'..., because the first stderr line of a clone is the temp directory. Nothing is deleted because of this; it only changes the message. The fix is \bssl(?:\b|_) in place of \bssl\b (it still leaves openssl in a path alone), plus that line as a case in the classifier test.
  2. CLAUDE.md:247 and docs/architecture-invariants.md:471: the new rule text is in the wrong file. The CLAUDE.md sentence carries mechanism and history (GitRun.started, "a folder named openssl once classified as unreachable"), its last clause ("and when the mount is unresponsive") does not attach to anything, and it nests the Docker refusal under the path clause although it applies to both branches. CLAUDE.md keeps the rule and architecture-invariants keeps the mechanism and the history, and the architecture-invariants section gained neither the Docker refusal nor the cleanup premise. I will shorten the CLAUDE.md sentence and move the detail into the architecture-invariants section.
  3. src/web/routes/case-routes.ts:751: two refusal messages point at the wrong tab. A path that is the cases folder, or directly inside it, gets prepareNewCasePath()'s reason, which says to use "plain Create New (no custom folder)". In Clone Repo that should say to clone without a custom folder. I will reword it there, the same way the route already rewrites the EXISTS reason.

Thanks again, this is a clean addition and the careful repro tests make it easy to trust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants