d1dbb6b8ea32042ada080ec9fa65099f52ad91bf
87 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Carlos Florêncio
|
2c293b55b0 | fix: preserve archived task resume state (#3115) | ||
|
Alassane
|
898225214b |
fix(worktree): reject stale task path reuse (#3013)
Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com> |
||
|
Carlos Florêncio
|
527cf1e9fa | fix(worktree): fail closed on required repository refresh (#3023) | ||
|
Alassane
|
0a45728753 |
fix: reuse task workspaces for additional sessions (#2843)
Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com> |
||
|
Carlos Florêncio
|
94fd9dcd44 | fix: classify expected runtime errors correctly (#2950) | ||
|
Carlos Florêncio
|
d99092e09c | fix: add durable task launch failure recovery (#2832) | ||
|
João Salavisa
|
a0ef8da13c |
fix(worktree): keep the local base branch when the pre-worktree pull fails (#2654)
Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
João Salavisa
|
382d60d704 |
fix(worktree): keep unpushed commits when the fetch retry hits a checked-out branch (#2658)
Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
15788438b4 | feat: route managed Improve Kandev contributions to bound forks (#2569) | ||
|
José Almeida
|
f218880ecb | feat(plugins): add source-control extension contracts (#2117) | ||
|
João Salavisa
|
a69a026741 |
fix(backend): recreate the worktree when resuming an unarchived task (#2508)
Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
João Salavisa
|
ac54f96e44 |
test(backend): assert three mutation-surviving code paths (#2481)
Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
Carlos Florêncio
|
2832170eb4 | fix(task): preserve task-owned worktrees across session deletion (#2456) | ||
|
Carlos Florêncio
|
acfbf58646 | fix: harden remote contribution launch safety (#2294) | ||
|
Carlos Florêncio
|
84fe8132a1 | feat: support remote contribution tasks (#2265) | ||
|
Carlos Florêncio
|
c6dffa122a | fix: prevent permanent owned-link target mismatch on task launch (#2253) | ||
|
Carlos Florêncio
|
3f46da6645 | fix(backend): harden office-disabled cron, title limits, and dead-runtime cleanup (#2206) | ||
|
Carlos Florêncio
|
431e5d40fe | fix: enforce classified Git admission paths (#2150) (#2181) | ||
|
João Manso
|
ca328f750f | fix(worktree): stop planting self-referential links in local repositories (#2007) | ||
|
João Salavisa
|
2850086db0 |
fix(worktree): export executor-profile env into repo setup script (#1971)
* fix(worktree): export executor-profile env into repo setup script Executor-profile environment variables (with secrets revealed) were merged into the launch request env and reached the agent subprocess, but the repository setup script — run inside worktree.Manager.Create, where `npm install` typically runs — was built with only the install-managed env (GOCACHE). The profile env was never injected, so tokens like an npm auth token were unavailable during install. This affected both single-repo and multi-repo tasks, since both run their setup script through Create. Thread the resolved profile env into the setup script at the point both paths converge: add CreateRequest.ScriptEnv (transient, never persisted so secrets stay out of the DB), set it from req.Env in buildWorktreeCreateRequest (used by single-repo and per-repo in multi-repo), and merge it over the managed env in runWorktreeSetupScript. Managed GOCACHE keeps precedence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(worktree): avoid overflow-flagged alloc size, clarify env precedence Address PR review: drop the len(a)+len(b) capacity hint in mergeScriptEnv (CodeQL flagged the addition as a possible allocation-size overflow) and reword the ScriptEnv doc comment to state that install-managed values such as GOCACHE take precedence over profile values on key collisions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(worktree): round out mergeScriptEnv edge cases Add profile-only and managed-only assertions to TestMergeScriptEnv per PR review, making the table exhaustive alongside the existing (nil,nil) and (profile+managed) cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
bc218dfbbe |
feat: add workspace sources from files panel (#1900)
* feat: add workspace sources from files panel * fix: preserve worktree identity during recovery * feat: unify workspace source actions * fix: validate recovered workspace repositories * fix: harden workspace source filesystem boundaries * test: split workspace repository chip coverage * fix: restrict repository rescan recovery paths * fix: stabilize workspace source CI checks * test: canonicalize workspace source paths * test: align workspace source container paths * fix: populate direct workspace repository names * test: preserve git fixture rewrites across restarts * test: isolate backend git fixture config * fix: resume remote workspace executions before creation * fix: preserve ssh agent identity during resume * fix: preserve SSH workspace across backend restart * fix: simplify workspace actions trigger * fix: refine add workspace repositories UX * docs: narrow workspace source UX updates * fix: avoid phantom turn on session resume * test(web): normalize workspace source request headers * feat(web): streamline workspace source picker * docs: remove trailing spaces from workspace source plan * fix: prevent resume status from creating phantom turns * fix(web): stabilize workspace source attachment * refactor(web): extract workspace source store contract * fix: avoid phantom turns on idle agent resume * fix: address backend static analysis findings * fix: preserve agentctl readiness status value * fix: preserve resume messages without active turns * fix: resume agents from promoted workspace root * chore: merge origin/main into feature/add-new-repo-folder-d97 * feat(web): explain workspace source consequences * test(websocket): count session activity subscription |
||
|
Carlos Florêncio
|
1f905be98f | fix: support long Git worktree paths on Windows (#1878) | ||
|
José Almeida
|
c645885cb5 |
fix(tasks): preserve inherited workspaces on subtask deletion (#1840)
* fix(tasks): preserve inherited workspaces on subtask deletion * fix(tasks): avoid archive workspace ownership transfer * fix(tasks): serialize shared environment handoff * test(tasks): cover blocked shared environment handoff * test(tasks): cover last-member environment cleanup * fix(tasks): roll back shared environment ownership |
||
|
Carlos Florêncio
|
77529e1eab | fix: preserve shared worktrees during task cleanup (#1819) | ||
|
João Salavisa
|
ac8401b709 |
fix: harden submodule init against malicious .gitmodules (#1802)
* fix: harden submodule init against malicious .gitmodules initSubmodules ran a bare `git submodule update --init --recursive` in a worktree whose HEAD is attacker-controlled during a fork-PR review task, with no protocol restriction, no hooks disable, and no cmd.Env. Attacker `.gitmodules` could pick a command-executing transport (`ext::sh -c`, CVE-2018-17456), a local `file://` path, or a plain-http URL (cloud metadata SSRF). Harden the single sink so all call sites benefit: - protocol.allow=never default-denies every transport; only https/ssh are re-enabled (blocks file://, git://, plain http:// SSRF). - protocol.ext.allow=never hard-pins the RCE transport off; on the command line it outranks any ambient protocol.ext.allow=always. - core.hooksPath=/dev/null neutralizes attacker-planted hooks. - GIT_TERMINAL_PROMPT=0 stops attacker URLs from hanging on a prompt. Residual: attacker-chosen https/ssh URLs are still fetched (transports real submodules require); fully closing that needs gating submodule init off for untrusted worktrees, tracked separately. * fix: address submodule hardening review feedback - Reuse newNonInteractiveGitCmd so submodule init inherits the full non-interactive env set (GIT_SSH_COMMAND=ssh -oBatchMode=yes, SSH/GCM askpass suppression, WaitDelay). Prevents an attacker ssh:// submodule URL from hanging the review runner on host-key/passphrase prompts until the context deadline (GIT_TERMINAL_PROMPT=0 alone only covers git's HTTP prompt, not SSH). - Correct the file:// comment: command-line -c outranks repo/global git config, so ambient protocol.file.allow=always does NOT re-enable file submodules here. This is intentional for untrusted trees; a trusted opt-in must be an explicit per-repo -c, not implicit host config. - Add a file:// URL variant to the transport-block test alongside the bare-path form (CVE-2022-39253 classification differs by git version). --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
João Salavisa
|
19c3771cf0 |
fix: validate destination parent chain in copyfiles byte copy (#1801)
* fix: validate destination parent chain in copyfiles byte copy copyState.copyFile wrote into the target worktree via os.MkdirAll + os.Create with no destination-parent symlink validation, unlike the sibling paths that were hardened for exactly this: symlinkMatch calls validateParentChain and remote writeOneEntry calls secureDestination. A worktree whose HEAD ships a directory segment as a symlink (a fork-PR head can set a tree entry to mode 120000 pointing at an absolute host path) let MkdirAll follow the link and os.Create write the source file's bytes outside the worktree as the backend user — a zero-click, content-driven arbitrary-file-write reachable on the PR-review path (copyConfiguredFiles -> copyfiles.Copy during worktree creation). Route the byte copy through the same secureDestination hardening: it runs validateParentChain, creates the parent dirs, and re-confirms containment via EvalSymlinks before returning a destination guaranteed inside the worktree. A rejected destination emits a non-fatal warning, mirroring symlinkMatch. The leaf os.Lstat skip-if-exists behavior is preserved (only a symlinked parent was the escape vector). Add byte-copy regression tests: a symlinked parent is rejected and nothing is written outside the worktree, a leaf symlink is still skipped, and a clean nested copy still works. * fix: canonicalize copyfiles target root before containment checks copyFile passed the raw s.targetDir into secureDestination, which EvalSymlinks-resolves the created parent and then Rel-checks it against that root. When the worktree is reached through a symlinked ancestor (macOS /tmp -> /private/tmp, or a user-symlinked tasks dir) the canonical parent looks like it escapes the non-canonical root, so every legitimate byte copy is silently rejected. Resolve targetDir once in Copy (falling back to the raw path when it does not exist yet), mirroring the EvalSymlinks WriteEntries already performs. Add a regression test that routes the target through a symlinked ancestor and asserts the copy still lands at the real location. --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
Carlos Florêncio
|
21f811d5ec | feat: add idle storage maintenance (#1699) | ||
|
João Salavisa
|
7bcb10bfef |
feat: unarchive tasks with worktree branch recovery (#1687)
* feat: unarchive tasks with worktree branch recovery Archived tasks can now be restored. Unarchive works for manual/legacy archives (empty cascade id, root-only) and cascade archives. Archive deletes the local branch, so worktree recreate now falls back to fetching the branch from origin, and unarchive writes the old branch back as checkout_branch so a fresh session resumes the prior work. Unrecoverable branches (never pushed) surface a warning. WS archive now routes through the handoff cascade so it stays stamped and reversible. UI: unarchive action on /tasks archived rows and the task top bar. * fix: address PR review feedback on unarchive recovery Forward PRNumber to the recreate fetch so fork-PR heads (only reachable via pull/<N>/head) are recoverable, publish task.updated per task in the cascade unarchive loop so the UI restores without a reload, and re-publish task.updated after a checkout_branch restore since the unarchive event goes out with the pre-restore repository rows. * fix: harden unarchive recovery per review feedback Propagate branch-probe errors instead of misreporting recoverable work as gone, and reserve ErrBranchUnrecoverable for a confirmed-missing remote ref (transient fetch failures keep their own error). Validate the archive root exists so WS/HTTP archive no longer reports success for unknown task IDs, CAS-guard manual unarchive on an empty cascade stamp, detach branch recovery from the request context, and skip checkout_branch restore for multi-branch repos. UI: in-flight guard on the list unarchive button, pluralized unrecoverable-branch toast. * docs: document CAS guard on UnarchiveTask interface contract --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
Carlos Florêncio
|
892ce9bf09 | fix(worktree): preserve colon paths in copy files (#1674) | ||
|
Jérémy Coatelen
|
fd7c7a2b0f | feat(worktree): add per-file copy/symlink materialization per repository (#1650) | ||
|
Carlos Florêncio
|
ff2647bf13 | feat: add configurable branch names (#1615) | ||
|
Carlos Florêncio
|
211e738ec0 | fix: reuse worktrees for same-task handoffs (#1510) | ||
|
Carlos Florêncio
|
a28c4226de | fix: make schema migrations replayable (#1489) | ||
|
João Salavisa
|
de334186bc |
fix(worktree): rmdir empty task parent after worktree removal (#1267)
* fix(worktree): rmdir empty task parent after worktree removal (#1266) removeWorktreeDir tore down only the inner {repoName}/ subdirectory under {tasksBase}/{taskDirName}/, leaving the now-empty parent task directory behind on every archive, delete, and rollback path. Husks accumulated under ~/.kandev/tasks/ over time. Now also attempts rmdir on the parent when it is a direct child of TasksBasePath. os.Remove no-ops if the parent still has siblings or workspace-scoped content, so multi-branch siblings and the tasksBase itself are preserved. * fix(worktree): normalize tasksBase path + cover multi-branch sibling case Address PR #1267 review feedback: - Apply filepath.Clean to ExpandedTasksBasePath() before comparing against filepath.Dir(parent). A configured TasksBasePath with a trailing slash or doubled separators previously fell through the equality guard, silently re-introducing the issue-1266 husk for those users. - Use cfg.ExpandedTasksBasePath() in the regression test for consistency with production code. - Add two test cases: - multi-branch sibling worktrees share a parent; removing one preserves the parent, removing both clears it. - trailing-slash TasksBasePath still triggers cleanup. * test(worktree): check ExpandedTasksBasePath error in tasksbase survival assert --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
João Salavisa
|
be9821fc55 |
fix(task): surface add_branch_to_task failures instead of silent orphans (#1264)
* fix(worktree): promote FallbackBaseBranch when BaseBranch is empty CreateRequest.Validate previously hard-rejected an empty BaseBranch even when the caller supplied a FallbackBaseBranch. The fallback is plumbed specifically so a missing primary branch can degrade into the repo's default — this lets that path actually fire. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(task): probe provider default branch in add_branch resolve path Adds an opt-in ProviderDefaultBranchProber seam (impl wired in cmd/kandev) and threads a ResolveProviderDefaults flag through TaskRepositoryInput so the add_branch_to_task GitHub-URL path can resolve a real default_branch before writing the Repository row. Also backfills the existing repo's default_branch when the probe succeeds and the workspace already had a row with an empty value. The create_task path keeps its zero-default contract (executor still backfills after clone). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(task): surface add_branch_to_task failures instead of silent orphans AddBranchToTask used to return a success envelope even when the post-insert worktree materialize step failed on a live task. The task_repositories row stayed in place with no on-disk worktree, and provider-URL flows also left an orphan repositories row behind. Three coordinated changes: - Gate empty base_branch after resolution with a typed error so no row is written when nothing can be materialized. - Track whether the repository row was created by this call and roll it back on any subsequent failure. - materializeBranch now returns its error; on a live worktree-executor task, AddBranchToTask rolls back the task_repositories row and surfaces the failure. Pre-launch tasks keep the legacy best-effort path so the next session launch still materializes via the multi-repo prepare flow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(mcp): classify add_branch base_branch resolution failure as validation AddBranchToTask now returns a "cannot resolve base_branch" error when the provider repo's default branch could not be probed. Map it to ErrorCodeValidation so MCP agents see a user-fixable input issue rather than an opaque internal error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(task): defer task.updated until after worktree materializes Publishing task.updated before materializeBranch ran meant a failed materialize on a live task would push the new task_repository row to WS clients and then silently roll it back, leaving stale UI state until the next poll. Move the event after materialize success so the bus only sees durable rows. Test now also asserts no task.updated fires on the rollback path. Renames stubProber.lastProvid to lastProviderArg for clarity. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(task): emit repository.deleted on add_branch orphan rollback Cleanup callback previously called repoEntities.DeleteRepository directly, which soft-deletes the row but bypasses Service.DeleteRepository's event publish. The create path emitted repository.created, so skipping the delete event left WS subscribers and frontend caches holding a phantom row after a resolve-then-fail sequence. Route through the service method so the events stay symmetric. Test now asserts the symmetry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(task): skip provider probe when workspace already has the repo resolveRepoInput's add_branch probe fired on every add_branch_to_task call with a github_url, even when the workspace already had the provider repo registered with a non-empty default_branch. The existing row's value would win seconds later via the repo.DefaultBranch fallback, so the remote git ls-remote round-trip was pure waste — and unnecessarily hit network on every call. Peek at the workspace registry first and short-circuit when a usable default_branch is already on file. Also adds a QA-tier test file covering probe-error fallthrough, bad URL rejection, registered-repo probe skipping, empty-default backfill, and the repository_id fast path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(task): detach rollback paths from caller ctx, log probe-lookup errors Two adjacent robustness fixes flagged on PR #1264: 1. resolveRepoInput silently discarded the GetRepositoryByProviderInfo error before the provider probe. A transient DB failure was indistinguishable from a clean "not found" lookup, so the code would pay for a network probe instead of failing fast. Surface the error in logs and skip the probe — FindOrCreateRepository below already hits the same DB and will return the real cause. 2. The repository-create cleanup callback in resolveBranchAddRepoRef and the task_repositories rollback in AddBranchToTask both captured the caller ctx for their DELETE queries. Rollback often runs precisely because that ctx was cancelled (timeout, client disconnect), and reusing it would kill the cleanup and orphan the row we just created. Use context.WithoutCancel so the rollback survives ctx death. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(task): close orphan-row gaps from caller cancel and concurrent create Two findings from Claude's review on PR #1264: 1. (blocker) materializeBranch called taskAlreadyLaunched(ctx, ...) after MaterializeBranch returned, using the same caller context that often failed the materialize in the first place. A cancelled ctx made the post-failure DB lookup return (nil, err), taskAlreadyLaunched returned false, and the live-task rollback path silently demoted itself to the pre-launch best-effort branch — leaving the orphan task_repositories row the PR exists to prevent. Snapshot the launch state BEFORE MaterializeBranch (launch state can't change during the synchronous call) so the rollback decision is anchored to a live-ctx answer. 2. (suggestion) resolveBranchAddRepoRef inferred "this call created the repo" by comparing the resolved ID against a workspace ListRepositories snapshot taken before FindOrCreateRepository. A concurrent add_branch for the same GitHub URL could create the row between snapshot and lookup; the second request would treat it as "not in my snapshot, must be mine" and register cleanup against the other request's data. On any later failure, cleanup would DELETE a repository this call didn't create and emit a spurious repository.deleted event. Thread an explicit `created bool` out of CreateRepository through FindOrCreateRepository → resolveRepoInput → ResolveRepositoryRef so the cleanup callback fires only when this call physically inserted the row. The flag is set at the CreateRepository / FindOrCreateRepository sites themselves and cannot be spoofed by concurrent activity. resolveRepoInput's branches were extracted into resolveRepoInputGitHub / resolveRepoInputLocal helpers to keep the parent function inside the cyclomatic-complexity and cognitive-complexity budgets after the signature change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: retrigger cubic CI --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
570c747959 | feat(worktree): support ** and brace globs in copy_files patterns (#1248) | ||
|
Carlos Florêncio
|
a8db7478ac | fix(backend): cap gh/git fork concurrency via shared subproc throttles (#1216) | ||
|
João Salavisa
|
fd79d6170f |
feat(tasks): multi-branch tasks — N (repo, branch) pairs per task (#1226)
* feat(tasks): multi-branch tasks — N (repo, branch) pairs per task Extends multi-repo into multi-branch so a single task can hold multiple worktrees on the same repo (one per branch). Sibling worktrees layout at <task>/<repo>/ + <task>/<repo>-<slug>/, dedicated PRWatch per branch, live agentctl tracker rescan + frontend store updates so the UI splits PRs / files / commits per branch without a session restart. MCP add_branch_to_task_kandev tool is shielded to the worktree executor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(web): unify multi-branch repo label as <repo> · <branch> across panels PR Changes and Commits panels rendered repo labels differently for multi-branch tasks — Commits showed "kandev · feature/x" while PR Changes leaked the raw "kandev-feature-x" agentctl tracker tag. Reformat tracker-tagged repo names via formatRepoLabel using the longest known-repo prefix so kandev-cli-* doesn't collapse to kandev · cli-*. * fix(tasks): address multi-branch PR review — data races, slug collisions, MCP error codes Backend - agentctl process.Manager: add repoTrackersMu RWMutex guarding repoTrackers + workspaceTracker. Reads snapshot under RLock; RescanRepositories builds trackers first, takes Lock for swap/append. Validates newWorkDir via os.Stat before publishing cfg.WorkDir. - worktree.Manager: reject invalid explicit BranchSlug up-front; cache key uses sanitized wt.BranchSlug so read/write paths are symmetric. - worktree.store: narrow isDuplicateColumnError to "duplicate column name". - task service AddBranchToTask: slug-collision guard rejects feat/a vs feat-a. - orchestrator resolvePushRepo: two-pass exact-name match before subdir prefix fallback so "backend" doesn't swallow pushes for "backend-admin". - executor buildRepoSpecs: duplicate repos always get a non-empty BranchSlug (falls back to branch-N when both branch fields sanitize to empty). - MCP handler: classifyAddBranchError maps service errors to NOT_FOUND / CONFLICT / VALIDATION_ERROR; add ErrorCodeConflict constant. - MCP server: extract registerAddBranchToTaskTool, register in task mode only (external mode contract: config tools + create_task_kandev). - workspace_rescan handler: reject malformed JSON with 400 instead of silently rescanning the current root. - branch_materializer: split MaterializeBranch into prepare + finalize helpers to satisfy the 80-line cap. Frontend - existingPrUrl returns undefined when a task has >1 PR so per-repo View PR buttons take over (no silent linking to a sibling). - formatRepoLabel checks exact known-repo match before prefix splitting so "kandev-cli" isn't mislabeled as "kandev · cli". - Repo chip key now includes base_branch so siblings differing only on base render as distinct chips. Docs - multi-branch spec uniqueness wording aligned with the canonical 4-column key (task_id, repository_id, base_branch, checkout_branch). - worktree.CreateRequest.BranchSlug doc describes the sibling layout (<repo>-<slug>) instead of the obsolete nested form. * fix(tasks): scope slug-collision check to repo only + use errors.Is for ErrTaskNotFound - service_branches: drop base_branch from the slug-collision check. Worktree path is (repo, slug); two rows on the same repo cut from different bases still collide on disk. - service_branches: prepareBranchAdd wraps taskrepo.ErrTaskNotFound via fmt.Errorf with %w so classifiers don't have to substring-match the formatted task UUID. - mcp handlers: classifyAddBranchError uses errors.Is(err, ErrTaskNotFound). * fix(agentctl): sanitize rescan work_dir before os.Stat to clear CodeQL path-injection CodeQL flagged the rescan endpoint as a path-injection sink (the HTTP-supplied work_dir flowed straight into os.Stat and downstream scanRepositorySubdirs). Add validateRescanPath which: - requires the input to be a clean absolute path with no ".." segments - rejects anything that isn't equal to or an ancestor of the current cfg.WorkDir — the only legitimate use case is the branch materializer promoting <task>/<repo>/ to <task>/ Defense in depth: agentctl binds to localhost, but any colocated process could otherwise direct it at /etc or the user's home by POSTing /api/v1/workspace/rescan. * fix(agentctl): tighten validateRescanPath, close TOCTOU on existingTrackers, add tests - Drop the strings.Contains(clean, "..") guard. filepath.Clean already resolves traversal; the substring check rejects legitimate names like /home/u/..hidden without adding defense (the ancestor check via filepath.Rel is the real boundary). - Drop the rel == "." branch in the ancestor check — early return for clean == currentClean already covers identity. - Read existingTrackers inside the write-lock section (not under RLock then re-Lock) so two concurrent rescans can't both see 0 trackers and double-bootstrap the multi-repo set. - Add table-driven TestValidateRescanPath covering rejection paths (relative, descendant, sibling, traversal-cleaned, empty input), legitimate ancestor promotion, exact match, no-anchor first-launch, and a /home/u/..hidden dotted-segment case. * fix(agentctl): break taint flow into os.Stat — derive rescan path from cfg, not HTTP body CodeQL's go/path-injection trace still flagged manager_rescan.go after the prior sanitizer because the user-supplied newWorkDir flowed (after validation) into os.Stat and then into cfg.WorkDir → tracker paths → workspace_monitor os.Stat sinks. Replace validateRescanPath with resolveRescanPath: it returns a path DERIVED from currentWorkDir (no-op = currentClean, promotion = filepath.Dir(currentClean)) and uses newPath only for comparison, never as a source for the returned string. os.Stat now receives a path whose provenance is trusted manager config. Functionally the legitimate caller (branch materializer promoting <task>/<repo>/ to <task>/) is unchanged. * test(agentctl): make TestResolveRescanPath windows-portable The Linux Backend Tests job passed but the Backend (windows) job rejected every "absolute" case because filepath.IsAbs("/task/repo") is false on Windows (no volume letter). Build inputs via filepath.Join against absRoot() (filepath.Abs of "/" / "\\") so the same table covers POSIX (`/...`) and Windows (`C:\...`) without per-OS branches. Keep the traversal-after-Clean case POSIX-only since filepath.Clean is the shared primitive and that input form needs a volume to be considered absolute on Windows anyway. * fix(agentctl): serialize RescanRepositories + re-check trackers under write lock Two concurrent RescanRepositories calls could both observe an empty tracker set (between the snapshot read and the bootstrap branch) and both enter transitionToMultiRepoMode, leaking a duplicate bare-root tracker and duplicate per-repo trackers. Same race in appendNewRepoTrackers would double-append the same new child across both callers. Fix: - Add rescanMu sync.Mutex on Manager. Acquired for the full body of RescanRepositories so the snapshot → branch → mutate sequence is serialized. repoTrackersMu still guards individual field reads from unrelated goroutines (subscribe/unsubscribe/poll-mode). - appendNewRepoTrackers re-checks membership inside the write-lock before appending and drops already-present trackers (Stop + detach outside the lock). Defense in depth. - Test: add `dotted segment is legal as promotion` case so the ..hidden-segment promotion path stays exercised. --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
9bb9644684 |
fix(backend): make repository setup script failures non-fatal (#1153)
* fix(backend): make repository setup script failures non-fatal
A failing repository setup script aborted environment preparation and
stopped the agent ("Environment setup failed"), and Resume / Start-fresh
couldn't recover because each relaunch re-ran the same failing script.
runWorktreeSetupScript (run inside worktree creation) treated a non-zero
exit as fatal: it deleted the worktree and returned an error, which
cascaded to EnvPrepareResult{Success: false} and aborted the launch. This
contradicted the already-non-fatal task/profile-level setup script path.
The repo-level script is now non-fatal too: on failure it logs a warning,
records SetupScriptWarning on the worktree, keeps the worktree, and lets
the launch proceed. The env preparer surfaces it on the "Create worktree"
step so the panel shows completed_with_warnings instead of failed. The
full script output still streams to the script_execution chat message.
Adds backend regression tests and an e2e test (verified red before / green
after) asserting the agent still launches when the repo setup script fails.
* fix(backend): preserve base-branch warning when setup script also fails
Addresses review feedback: the post-Create block surfacing SetupScriptWarning
on the "Create worktree" step unconditionally overwrote BaseBranchFallbackWarning
when both occurred, silently dropping the "wrong branch" notice. Append instead
of replace so both survive.
Also updates TestWorktreePreparer_SetupScriptFailure_KeepsCreateWorktreeStepCompleted
to the non-fatal contract: a failed setup script now yields a successful prepare
with a warning on the step, not a failed prepare.
|
||
|
João Salavisa
|
cab7b28ed9 |
chore: round 4 dead-code cleanup (transport helpers, detect, orphan methods) (#1156)
* chore(agentctl): delete duplicate procattr files in adapter package setProcGroup and killProcessGroup are defined in both agentctl/server/adapter/ and agentctl/server/process/. The process/ copies are the live ones (called by process/runner.go, manager.go, vscode.go); the adapter/ copies have zero callers and are removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(agentctl): remove orphan helpers from transport/shared Left over from the ACP refactor. All had zero callers: - DebugEnabled(): internal debugMode var is checked directly by LogRawEvent/LogNormalizedEvent — the exported wrapper was unused. - GetBool, GetMap, GetSlice: only GetString/GetInt are called in acp/normalize.go. - NormalizeShellResult, NormalizeReadResult, NormalizeCodeSearchResult, NormalizeModifyResult: normalization moved to acp/normalize.go. - TruncateIfNeeded, MaxContentLength, MaxFileCount: only used by the four Normalize* helpers above; removed transitively. - TraceControlMessage: zero callers in the adapter transport layer. GenerateUnifiedDiff, SplitLines, DetectLanguage, Tracer, ShutdownTracing, TraceProtocolEvent, TraceProtocolRequest are alive and kept. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(agentctl): remove unused tool_payload constructors NewHttpRequest, NewCreateTask, NewSubagentTask, NewShowPlan, NewManageTodos, and NewMisc have zero callers — ACP normalization builds NormalizedPayload via struct literals and JSON unmarshaling, not these factory functions. NewReadFile, NewModifyFile, NewShellExec, NewCodeSearch, and NewGeneric are kept (live callers in acp/normalize.go). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(agents/detect): remove dead fluent options and OSPaths type WithFileExists, WithCommandOutput, WithEnvVar, expandHomePath (their transitive helper), and OSPaths (struct + Resolve + Expanded) have zero callers. Only WithCommand and WithNpxRunnable are used by agent IsInstalled implementations. Imports trimmed to context and os/exec. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(office): remove dead channel methods superseded by service/ ListComments, DeleteComment, and UpdateChannelStatus on ChannelService have zero callers — the live implementations live in internal/office/service/comments.go and service/channels.go. CreateComment is kept: it is called by HandleChannelInbound. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: remove scattered orphan helpers Remove dead single-symbol exports verified to have zero callers: - ParseCronExpression (office/shared/cron.go): NextCronTime covers all live usages; the validator wrapper was unused. - IsValidPathComponent, MustJSON (office/shared/helpers.go): zero callers; GenerateSlug is kept. - DiscoverUserSkillsAt (office/skills/userhome_discovery.go): exported wrapper over the private discoverUserSkillsAt; only the internal DiscoverUserSkills method is wired up. - Manager.Get (agent/loginpty/manager.go): zero callers; GetByID and Stop are kept (Stop is tested in manager_test.go). - DetachedWithValues (common/appctx/appctx.go): zero callers; Detached is kept. - Repository.Close no-ops (task/share, terminal/repository): both are documented no-ops with zero callers. - TaskBranchName (worktree/config.go): zero callers; the deterministic TaskBranchNameWithSuffix is kept. Note: dialect.Now was initially targeted but is tested in dialect_test.go (same-package call — missed by package-qualified grep). It remains unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(share): remove stale Close reference from provider.go comment Repository.Close() was removed as a no-op in the round-4 cleanup. The comment in provider.go still referenced it by name; updated to reflect that the repo simply doesn't own its DB connections. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(office/shared): delete helpers.go — GenerateSlug had zero callers The triage step mistakenly identified skills/service.go as a caller, but that package defines its own local GenerateSlug (different regex, adds TrimSpace and a 'skill' fallback default). shared.GenerateSlug was never called anywhere. helpers.go is now empty; delete the file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(worktree): fix stale TaskBranchName reference in TaskBranchNameWithSuffix doc Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
1fcc581a09 | fix(backend): complete Create worktree step before setup script runs (#1143) | ||
|
João Salavisa
|
62ddfd4d17 |
refactor(worktree): split manager.go into per-concern files (#1055)
The Manager type owned creation, removal, state queries, and git helpers in a single 1,615-line file (2x the lint threshold). Decompose into five files within the same package, preserving the Manager public API and all existing test coverage: - manager.go struct, interfaces, NewManager, repo locks - manager_lifecycle Create, recreate, git worktree add paths - manager_state cache, persistence, Get* lookups, IsValid - manager_cleanup RemoveByID, setup/cleanup scripts, dir removal - manager_git branch inspection, non-interactive git, pull sync Extract four helpers to satisfy the new-code lint baseline that fires on moved-but-unchanged complexity hot-spots: - tryReuseExisting + resolveBaseRefWithFallback split Create (cyclo 21) - gitAddWorktreeForRecreate trims recreate below 80 lines - retryFetchAsRemoteTrackingRef flattens fetchBranchToLocal nesting No behavior change; all existing internal/worktree tests pass unchanged. Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
3b1b3473f1 | fix(web): avoid trailing-context crash in multi-file diff panel (#1097) | ||
|
João Salavisa
|
7be0db04d8 |
fix(backend): restrict branch/dir name sanitizers to ASCII alphanumerics (#1095)
* fix(backend): strip non-ASCII chars from branch/worktree names Non-ASCII titles (CJK, Cyrillic, Arabic, emoji) flowed verbatim into git branch names and worktree directory names because the sanitizers used unicode.IsLetter/IsDigit, which accept any Unicode letter or digit. The resulting names broke many downstream tools and left worktrees effectively inaccessible. Restrict SanitizeForBranch and SanitizeRepoDirName to ASCII alphanumerics. Non-ASCII runes now route through the existing hyphen-replace + collapse + trim pipeline, and the existing fallback chain (sanitized title -> sanitized task ID -> "task") handles titles that become empty after stripping. The "Issue #N:" / "PR #N:" prefix already added by the GitHub ingestion path means a non-ASCII issue title still produces a recognizable name like "issue-123". Closes #1081 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(worktree): fix comment accuracy and harden test assertion - Remove "emoji" from isASCIIAlphaNum doc — emoji are not Unicode letters (unicode.IsLetter returns false), so the prior code never admitted bare emoji. Correct examples to CJK, Cyrillic, Arabic. - Replace hard-coded len(dirName) != 8 with strings.Contains check so the assertion stays correct if suffix length ever changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
adc13c6b5a | feat: copy gitignored files from repo into new worktrees (#946) (#950) | ||
|
Carlos Florêncio
|
c9b6fe4b8a | fix(worktree): support fork PR worktree creation via pull refspec (#990) | ||
|
Carlos Florêncio
|
0b2085688b | feat(office): autonomous agent management layer (#914) | ||
|
Carlos Florêncio
|
a5108de926 | fix(windows): make conpty Close idempotent and kill orphan agentctl (#900) | ||
|
José Almeida
|
f3397b4ad7 |
feat: docker and sprites executor improvements (#738)
* feat(orchestrator): generalize task environment reuse across executor types
Worktree, container, and sandbox IDs now flow uniformly from a persisted
TaskEnvironment back into LaunchAgentRequest through a single helper. This
gives Sprites session-to-session reuse the same first-class treatment that
Docker already gained from the GitHub-token-injection PR, and lets a fallback
launch refresh the environment row instead of leaving stale IDs in the DB.
- applyExistingEnvironment propagates WorktreeID, ContainerID (via metadata),
and SandboxID (as sprite_name metadata) onto the launch request
- persistTaskEnvironment refreshes those IDs when the executor ends up
creating new resources (e.g. the previous container was pruned)
- extractSandboxID lifts the sprite identifier out of executor response
metadata so it can be persisted on the TaskEnvironment row
* feat(backend): add task-level reset environment action
Users sometimes need a clean slate — broken packages, stale caches, a
container that drifted into a bad state. Reset Environment is the explicit
escape hatch: tear down the recorded container, sandbox, and/or worktree
for a task and delete the TaskEnvironment row so the next session starts
fresh.
- service.ResetTaskEnvironment(taskID, ResetOptions) is the entry point.
ResetOptions.PushBranch optionally git-pushes the workspace branch
before teardown so committed work isn't dropped on the floor; a failed
push aborts the reset and leaves the environment intact.
- A new EnvironmentDestroyer interface in the task service hides the
per-resource teardown behind a single seam, implemented in cmd/kandev
by delegating to lifecycle.Manager (containers, sprites) and
worktree.Manager (worktrees, branch preserved).
- lifecycle.Manager.DestroyContainer / DestroySandbox dispatch to the
Docker and Sprites executor backends without leaking model types into
the lifecycle package.
- Reset is blocked while any session on the task still has an
ExecutorRunning row (returns ErrSessionRunning → HTTP 409). Per-resource
teardown is best-effort but the DB row is only removed once everything
is confirmed gone or already missing, so a partial failure is retryable
by clicking Reset again.
- Endpoint: POST /api/v1/tasks/:id/environment/reset.
* feat(web): add executor settings popover with reset environment action
A new button on the task top bar opens a popover showing the active
TaskEnvironment — executor type, status, container/sprite/worktree
identifiers, and a copy-ready 'docker exec' command for local Docker.
The popover hosts the destructive Reset Environment action so users have
one obvious place to inspect and manage the environment without
cluttering the New Session dialog.
The reset confirmation dialog requires explicit acknowledgement that
uncommitted changes will be lost. When the environment has a host-side
worktree it offers an opt-in 'push branch before resetting' checkbox so
committed work can be pushed first; the backend aborts the reset on
push failure.
- task-environment-api.ts: fetchTaskEnvironment + resetTaskEnvironment
- executor-settings-button.tsx: popover trigger + identity panel
- task-reset-env-confirm-dialog.tsx: destructive confirm with push opt-in
- task-top-bar.tsx: wire the new button into the right-side controls
- e2e: replace fresh-env-toggle test with reset-from-popover flow
* fix(docker): expand {workspace} placeholder in container WorkingDir
The agent runtime config carries `WorkingDir: "{workspace}"` for most ACP
agents. Mount sources were already expanded via expandMountSource, but
WorkingDir was passed through verbatim, so Docker rejected new containers
with:
Error response from daemon: the working directory '{workspace}' is
invalid, it needs to be an absolute path
Run the same expansion on WorkingDir, defaulting to /workspace when no
host WorkspacePath is set (the clone-inside-container case introduced by
PR #654).
* fix(docker-profile): persist Dockerfile/image_tag and apply tag override at runtime
Two bugs:
1. The Dockerfile editor and Image Tag input on the executor profile page
stored their values in component-local useState inside DockerSections,
not in the form state. Saving the profile never wrote them back, so
"Use defaults" / typing in the editor / changing the tag never
persisted — and the Build button was the only thing that consumed
them. The same gap existed in the new-executor flow's
buildProfileConfig.
Lift `dockerfile` and `image_tag` into useProfileFormState (edit) and
useCreateProfileFormState (new), and emit them in buildSaveConfig /
buildProfileConfig so they round-trip through profile.config.
2. Even when an image_tag is set on the profile, the container builder
ignored it. imageName was always derived from the agent runtime's
hardcoded `kandev/multi-agent`, so a user who built a custom image
like `kandev/agent:latest` would still see Docker fail with
"No such image: kandev/multi-agent:latest".
Propagate `profile.config.image_tag` → metadata
(`image_tag_override`) → ExecutorCreateRequest → ContainerConfig,
and let it override `rt.Image` in buildContainerConfig when set.
Also align the frontend default image tag (kandev/multi-agent:latest)
with the backend's hardcoded runtime default so users who don't override
end up with a working build.
* test(docker): cover container config build + Docker profile persistence
Adds two regression tests for the recent Docker fixes:
- container_config_test.go (Go unit): asserts buildContainerConfig
expands `{workspace}` in WorkingDir (defaulting to /workspace when no
host WorkspacePath is set) and that ContainerConfig.ImageTagOverride
takes precedence over the agent runtime's hardcoded `rt.Image`. No
Docker daemon required.
- docker-profile-persistence.spec.ts (Playwright E2E): creates a
local_docker executor + profile, opens the profile edit page, clicks
"Use defaults" to populate the Image Tag input + Dockerfile editor,
hits Save, then refetches the profile via API and reloads the page to
verify both fields round-trip through `profile.config`. Catches the
prior regression where DockerSections owned the form state locally
and never wrote it back. Runs against the existing E2E backend that
has KANDEV_DOCKER_ENABLED=false — we only test the persistence path,
not container creation.
Adds an api-client helper (`getExecutorProfile`) for the round-trip
assertion.
* feat(docker): self-contained Dockerfile + mount agentctl + capabilities adapter
Make the Docker executor work with a Dockerfile users can actually build,
without depending on a maintainer-shipped image:
- The container now launches a small bootstrap (`sh -c "$KANDEV_PREPARE_SCRIPT
; exec /usr/local/bin/agentctl"`) regardless of the image's ENTRYPOINT.
The agent CLI is no longer wired through container Cmd; agentctl receives
it later via its CreateInstance HTTP API.
- Host agentctl-linux binary is mounted into the container at
/usr/local/bin/agentctl via the existing AgentctlResolver. User-built
images don't have to bake it in.
- Default Dockerfile in the executor profile UI replaced with a working
node:22-slim + git + curl image — runs npx-based ACP agents (Claude,
Codex, OpenCode, Auggie) out of the box.
- lifecycleAdapter now exposes RequiresCloneURL / ShouldApplyPreferredShell
so the orchestrator can correctly identify containerized executors and
resolve the clone URL. Without this, applyRepositoryConfig short-circuits
on `e.capabilities == nil`, the cloneURL is never resolved, and the
prepare script fails with `repository '{{repository.clone_url}}' does
not exist`.
- applyRepositoryConfig also surfaces the resolved clone URL to script
metadata so the prepare script can interpolate {{repository.clone_url}}.
- docker.ContainerConfig gains an Entrypoint field, plumbed to the docker
client.
Verified end-to-end on a local stack with the new minimal image: the
container starts, bootstrap clones the GitHub repo, agentctl launches and
hands off to claude-acp inside the container.
* fix(docker): patient health check + resilient bootstrap
Two follow-ups to the Docker bootstrap work:
- The 30 retries × 500ms = 15s budget for agentctl health was too tight.
In the previous flow agentctl was a separate exec from the prepare
script, so it came up immediately. Now it runs after the prepare
script (which does git clone, optional installs), so first launch can
easily exceed 15s and produce a misleading "agentctl not healthy after
30 retries" error. Bumped to 240 retries (120s) and surfaced the last
underlying error in the message so failures point to the real cause.
- Prepare scripts commonly start with `set -e`/`set -eu`; running them
via `eval` propagated the option to the outer bootstrap shell, so any
failure killed the bootstrap before `exec /usr/local/bin/agentctl`.
Run prepare in a subshell so its `set -e` is contained, and start
agentctl regardless. If prepare fails, agentctl still comes up — the
host can connect, surface the failure, and let the user debug from
the Executor Settings popover.
* fix(executor-profile): persist remote auth secrets/credentials for Docker profiles
The Remote Credentials card (per-agent secret picker, plus the GitHub
credential checkbox) is rendered for any remote executor — Sprites,
local_docker, or remote_docker — but the save path in both the create
and edit pages only emitted `remote_auth_secrets` and `remote_credentials`
when `isSprites` was true. Selecting a secret for the Claude / Codex /
Auggie ANTHROPIC_API_KEY-style entry on a Docker profile updated form
state, the dropdown showed it as picked, but Save Changes silently
dropped the value before posting to the API. On reload the agent showed
as "Not configured", and any session start failed with
`{"code":-32000,"message":"Authentication required"}`.
Switch the guards to `isRemote` so the same data persists for all remote
profile types. The backend orchestrator already propagates
`profile.config.remote_auth_secrets` and `remote_credentials` through to
metadata for any containerized executor and resolves them via
`applyContainerCredentials` → `resolveAuthSecrets`, so no backend change
is needed once the values actually land in the saved profile.
* fix(executor-profile): live Configured badge + session/load resilience + live container status
Three independent fixes that all surface together while iterating on the
Docker executor flow:
- Remote Credentials card: the per-agent "Configured" / "Not Configured"
badge was driven by useState that only seeded once on mount. Picking a
secret in the dropdown updated the parent's `agentEnvVars` map but the
local `choice` never moved to `"env"`, so the badge stayed grey until
the page was reloaded. Derive `choice` straight from props every render
so the badge tracks live state.
- ACP session/load: when the agent process restarted between turns
(common with claude-agent-acp because its session map is in-memory),
the host's session/load call failed with JSON-RPC -32002 "Resource not
found" and the entire launch errored out. Treat that as the same kind
of "session is gone" signal as -32601 / capability=false and fall back
to session/new. Worst case the agent loses prior context; best case
the user just keeps typing.
- Executor Settings popover: dropped Execution ID and Environment ID
rows (debug-only noise users don't need), kept Container ID + the
copyable `docker exec -it … bash` line, added a live status badge
with tones for running / exited / paused / dead / error / missing,
and a 3-second polling interval so the badge follows the real
container state. New `GET /api/v1/tasks/:id/environment/live`
endpoint plumbs Docker `inspect` data through the lifecycle manager
→ task service → handler. ContainerLiveStatus mirror type at the
service layer keeps lifecycle types out of the public surface.
* feat(web): gate agent profiles by executor profile credentials
Filter the agent profile dropdown in both the Create Task dialog and the
New Session dialog to only show agents that are actually configured for
the selected executor profile.
For local/worktree executors, no gating (agents need no extra creds).
For Docker/Sprites profiles, an agent is "configured" iff one of its
remote-auth methods has a value in `remote_credentials` or a non-null
secret in `remote_auth_secrets`. Agents without an entry in the remote
auth catalog (Copilot, Amp, OpenCode, TUI, mock) cannot carry creds on
remote executors and are blocked.
Spec IDs match `AgentProfileOption.agent_name` (the registry-type string
like "claude-acp"); `agent_id` is a DB UUID and unrelated to the catalog.
The Create Task footer surfaces a "No agent profile is configured for X"
tooltip and disables submit when the selected executor profile has no
compatible agents. The New Session dialog auto-swaps to the first
compatible agent and shows the same banner inline.
`useRemoteAuthSpecs` returns a `loaded` flag so callers don't filter
during the initial fetch (would otherwise hide everything).
* feat(docker): make mock-agent runnable inside Docker for E2E
Lay the backend groundwork so the existing mock-agent can run inside
local_docker containers — needed for real-Docker E2E tests that don't
require Anthropic/OpenAI API keys.
Container builder bind-mounts a host mock-agent binary at
/usr/local/bin/mock-agent when KANDEV_MOCK_AGENT_LINUX_BINARY is set
(or build/mock-agent-linux-amd64 sits next to the kandev binary), the
same pattern already used for agentctl. MockAgent.BuildCommand always
resolves via PATH so the same command works on host (E2E PATH includes
bin/) and in containers (mounted at /usr/local/bin/mock-agent).
Mock now declares non-nil empty RemoteAuth so it counts as a
remote-capable agent. The remoteauth catalog includes specs even with
zero methods, and the frontend gating helper treats an empty-method
spec as "no credentials needed" — letting mock-agent pass the
configure-on-executor check that hides uncatalogued agents on remote
executors.
repositoryCloneURL falls back to the local origin remote URL when the
repo has no provider info, so local-only repos with a real remote
(including file:// remotes used in E2E) work on remote executors that
clone inside the container.
Makefile: build-mock-agent-linux target mirrors build-agentctl-linux.
* test(docker-e2e): add real-Docker Playwright project + first spec
A new Playwright project — \`docker\` — runs container-bound E2E specs
against a backend with KANDEV_E2E_DOCKER=1. The fixture probes for a
Docker daemon and skips the worker entirely when none is reachable, so
contributors without Docker can still run the rest of the suite.
The fixture builds a tiny kandev-agent:e2e image once per machine
(node:22-slim + git/curl), pre-seeds a local_docker executor profile,
and seeds a local repo with a file:// origin so the executor's
clone-inside-container path resolves without network access. A small
WS-based session.launch helper is added to the api-client for tests
that need to launch additional sessions on an existing task.
First spec — docker-launch.spec.ts — covers the smoke path: a session
launches in a real container, the task environment records its
container_id, and \`docker inspect\` confirms the container exists.
Two follow-up specs (multi-session env reuse + recovery from external
container removal) are wired up but marked test.fixme with comments
pinpointing the backend gaps that block them: persistTaskEnvironment
isn't called for workflow-prepared sessions (so task_environment_id
stays empty on those rows), and DockerExecutor.reconnectToContainer
loses the agentctl handshake auth token across launches (so reconnect
401s and falls back to a fresh container every time).
Run locally with: \`pnpm --filter @kandev/web exec playwright test
--config e2e/playwright.config.ts --project=docker\`.
* ci(e2e): add dedicated Docker E2E job
The new \`docker\` Playwright project needs a Docker daemon and the
linux/amd64 agentctl + mock-agent binaries bind-mounted into containers.
Pin the existing sharded job to chromium + mobile-chrome so the docker
project no longer leaks into every shard (where it would fail seeding
without KANDEV_DOCKER_ENABLED), and add an \`e2e-docker\` job that:
- Verifies the Docker daemon is reachable on the runner.
- Downloads the same backend artifact (now including the linux helper
binaries, built via \`make build-backend-linux-helpers\`).
- Runs \`playwright test --project=docker\` with KANDEV_E2E_DOCKER=1.
- Cleans up any kandev-managed containers at teardown so leaks don't
poison the next run.
* ci(e2e): include docker job in merged HTML report
Upload the docker job's blob report under the \`blob-report-*\` glob the
merge job already consumes, and add e2e-docker to e2e-report's needs so
the final HTML report covers both the sharded chromium runs and the
docker run.
* ci(e2e): add workflow_dispatch for ad-hoc runs and draft-PR verification
* fix(build): always produce linux/amd64 agentctl as part of \`make build\`
Remote executors (local Docker, remote Docker, Sprites) bind-mount the
linux/amd64 agentctl binary into the container. On non-Linux hosts (e.g.
macOS dev installs) running plain \`make build\` produced only the host
binary, so the first Docker session would fail with:
failed to build container config: agentctl linux binary not found
The top-level \`make dev\` already special-cased non-Linux hosts to chain
\`build-agentctl-linux\`, but \`make build\` and \`make start\` skipped it.
Add it as a dependency of the standard backend build so every install
ships a usable linux variant. Cross-compiling agentctl is fast (~1-2s)
and the binary is ~20 MB, so the cost is negligible.
Users who hit the error can run \`make build-backend\` (or \`make build\`
in apps/backend) to regenerate; setting KANDEV_AGENTCTL_LINUX_BINARY
remains a manual override.
* fix(release): ship linux/amd64 agentctl in every release artifact
Remote executors (local Docker, remote Docker, Sprites) bind-mount the
linux/amd64 agentctl into the container. The release pipeline only
shipped a host-arch agentctl, so users who installed via the bundled
binaries (npm/Homebrew/etc.) hit the same "agentctl linux binary not
found" error the first time they tried a Docker executor — even on
Linux, since the resolver expects the -linux-amd64 suffix.
Changes:
- .github/workflows/release.yml: cross-compile agentctl-linux-amd64 in
the build-bundles matrix step (CGO off so it works from every host).
Bundle ships kandev + host agentctl + agentctl-linux-amd64 in
dist/kandev/bin/. The AgentctlResolver finds it next to the kandev
binary automatically.
- Dockerfile: same idea for the container image — produce both variants
in the Go builder stage and copy the linux-amd64 helper into
/app/apps/backend/bin/ so the resolver picks it up inside the image.
A previous commit added the same dependency to \`make build\` for local
dev installs; this commit closes the gap for end users.
* fix: address PR review feedback (CI, CodeRabbit, Greptile, Claude)
Backend:
- service_task_environments: fail closed when checkAnySessionRunning
errors so a transient DB hiccup can't bypass the running-session
guard and tear down a live container/sandbox; teardown is now truly
best-effort with errors.Join across container/sandbox/worktree, so a
failed Docker call doesn't permanently orphan the worktree on retry.
Adds tests for the running-check error path and the multi-resource
best-effort path.
- executor_execute: propagate GetTaskEnvironmentByTaskID errors instead
of silently launching a fresh environment that orphans existing
containers/worktrees; gate environment reuse on matching ExecutorType
so a profile switch doesn't pass stale PreviousExecutionID/container
IDs to the wrong backend.
- task_environment_handlers: reject malformed JSON on POST
/tasks/:id/environment/reset (still allow empty body) so a destructive
endpoint doesn't silently proceed with a bad payload.
- worktree (PushEnvironmentBranch): bound git push with a 30s context
timeout and disable interactive prompts via GIT_TERMINAL_PROMPT=0
so a stalled remote can't hang the request thread.
- manager_environment: explicitly call ensureClient() before
ContainerMgr() in DestroyContainer/GetContainerLiveStatus so a missing
Docker daemon surfaces as "docker unavailable" rather than the
generic "container manager not initialized".
- container: WorkingDir always uses the in-container /workspace path
rather than the host workspace path (Docker would happily start in an
unrelated directory in host bind-mount mode); bootstrap now logs the
prepare-script exit code so failures show up in container logs.
- docker/client: CreateContainerInteractive honors ContainerConfig.Entrypoint
so the same config struct produces consistent behavior on both creation
paths.
Frontend:
- executor-settings-button: extract useTaskEnvironment hook (component
shrinks below the 100-line limit); only treat 404 as empty env (other
errors no longer wipe a valid environment); track "loaded once" via a
ref so loadEnv doesn't depend on env state — that dep was creating a
new function identity on every successful poll, cancelling the
3-second interval and turning the popover into a tight loop hammering
/environment/live; copy-shell command uses sh instead of bash so it
works on user-built minimal images.
- task-reset-env-confirm-dialog: AlertDialogAction onClick calls
e.preventDefault() so Radix's auto-close doesn't dismiss the dialog
before the async reset completes (loading spinner now visible; close
is parent-controlled via onOpenChange).
- remote-credentials-card: track env method in selectedIds the same way
files/gh_cli_token are tracked, and let initialChoice match either
selectedIds OR envSecretId. First-time env-credential setup for an
agent that exposes both files+env was broken: clicking the env radio
never updated state, choice re-derived to "none", and the secret
picker disappeared before the user could pick anything.
- e2e/executor-reuse: select the acknowledge checkbox by label, not by
positional last(), so adding another checkbox to the dialog can't
silently target the wrong control.
Format pass on prettier-flagged files; gofmt fixed two whitespace
nits in lifecycle files.
* test(executor): cover applyExistingEnvironment executor-type mismatch guard
* fix(docker): connect agentctl through published ports
* fix(web): keep remote auth selection stable
* fix(docker): reuse task containers across sessions
* fix(docker): resume task environment after refresh
* fix(docker): track executor containers for cleanup
* fix(docker): explain missing agent credentials
* fix(docker): surface container launch progress
* fix(docker): stabilize container reuse and feedback
* fix(docker): show task titles in container lists
* fix(docker): use container icon for docker tasks
* fix(orchestrator): align environment reuse after rebase
* fix(sprites): reconnect sessions on existing sandboxes
* fix(docker): use container icon for local docker executor too
Local docker tasks were rendering the Docker brand "chip" mark while
remote docker uses the container glyph. Map both variants to
IconContainer so the executor surfaces look consistent.
* feat(sprites): provision a fresh sandbox when the persisted one is gone
Resuming a Sprites task whose sandbox was destroyed upstream returned
"sprite not found" and dead-ended the user. Detect the upstream 404 via
a typed ErrSpriteNotFound sentinel, mark the reconnect step skipped with
a user-facing warning explaining what happened, swap the prepare plan
in place to the fresh-provision shape, and continue the launch under a
new sandbox name on the same branch.
The progress plan is now mutable so the reporter can stream the fresh
steps live after the reconnect-then-fresh transition. Only the explicit
not-found case triggers fallback; other reconnect failures still
surface as before, and the fallback fires at most once per launch.
* feat(web): friendlier header for sandbox-recovered prepare runs
When the only warning on a completed prepare is the missing-sandbox
fallback notice, render "Environment prepared on a fresh sandbox" in
the header instead of the generic "Environment prepared with warnings".
The fallback row still carries its own amber warning callout so the
user understands why a fresh sandbox was provisioned.
* feat(web): unify executor surfaces in the right-side popover
The desktop task top bar carried a small cloud icon (RemoteCloudTooltip)
in the breadcrumb cluster while the right-side Executor popover showed
the same kind of info — two surfaces for one concept. Remove the cloud
icon from the desktop top bar; kanban cards, task list rows, and the
mobile top bar keep theirs since they have no popover.
The popover button now picks its icon by executor type (cloud for
sprites, container for docker, server for local PC) using a shared
getExecutorStatusIcon helper. While the prepare progress is preparing
the icon swaps to a spinner so the user can see at a glance that work
is in flight.
The popover content gains a Preparing-environment section that streams
"Step N of M: <step>", an amber inline note when the missing-sandbox
fallback is in progress, an Environment-ready row with duration on
success, and a failed-step summary on failure. Polling promotes to the
active 3s cadence while preparing so the icon flips to ready without
the user opening the popover.
* fix(docker): swap container glyph for a plain box icon
IconContainer's vertical ribbing reads as a microchip on small sizes.
IconBox is an unambiguous outlined cube. Use it for both docker
variants so the executor surfaces look like a box, not a chip.
* fix(orchestrator): preserve base branch on resume for clone-based executors
Sprites and Docker resumes were sending the launch request with an empty
BaseBranch, so the in-sandbox prepare script's `git clone --branch <X>`
shifted positional args and failed with "repository '/workspace' does
not exist". applyResumeRepoConfig only set BaseBranch inside the
worktree-specific block; widen that to all executors that require a
clone URL.
Local executors are intentionally left untouched: LocalPreparer reads
BaseBranch and forces a `git fetch && git checkout` against the user's
own workspace, which would clobber the "use current state" path.
* feat(sprites,docker): generate a kandev feature branch for clone-based executors
Sprites tasks committed straight onto the chosen base branch because
SpritesPreparer didn't populate WorktreeBranch and the orchestrator's
"forward env.WorktreeBranch on reuse" check only fired for Docker.
Mirror the Docker preparer's TaskBranchName generation in Sprites,
extract a shared `nonWorktreeTaskBranch` helper that uses a
deterministic suffix derived from the task ID (so legacy env rows that
predate this change resolve to the same branch on every resume), and
let the orchestrator forward env.WorktreeBranch for any clone-based
executor — Docker or Sprites.
* fix(sprites,docker): prefer existing remote feature branch on resume
When a sandbox is destroyed and we provision a fresh one for the same
task, the agent may have previously committed and pushed onto the
feature branch. The previous prepare scripts only ran `git checkout -b
{{worktree.branch}}` which would overwrite that progress with a fresh
branch off the chosen base.
Try `git fetch origin <feature>` first; on success, `git checkout -B`
against the remote tip so the agent picks up where it left off. Falls
back to a fresh local branch when origin has nothing.
Also add the same checkout block to the Sprites prepare script — it was
missing entirely, so every Sprites session was committing onto main.
* test(sprites): lock in informative branch name in fallback warning
Now that BaseBranch is preserved on resume for clone-based executors,
the missing-sandbox fallback warning detail names the actual branch
("Cloning … on the same branch main") instead of the "(unknown)"
placeholder. Add a regression test so a future change to the resume
metadata propagation can't silently regress that copy.
* fix(sprites): preserve sandbox on plain agent stop so resume is fast
StopInstance was calling sprite.Destroy() unconditionally, so every
session stop tore down the cloud sandbox. Resume then always fell into
the missing-sandbox fallback, re-cloned the repo, re-ran the prepare
script, and lost the agent's working tree. That's a 30s+ round-trip
for what should be "stop the process, then re-attach later".
Limit Destroy to terminal lifecycle reasons (task/session
deleted/archived), the same set runTerminalCleanupScript already uses.
Plain stops just close the proxy and leave the sandbox alive.
Also update the existing Docker e2e regex: the feature-branch suffix is
now a deterministic 6-hex-char prefix of the task UUID rather than a
random 3-char tail, so resume on legacy env rows lands on the same
branch.
* test(docker): cover stop + resume → same container, branch preserved
Adds a Docker e2e test that exercises the user-visible stop-then-resume
flow: launch session, stop it via WS session.stop, launch a fresh
session on the same task, and assert the same task environment +
container ID are reused and the kandev feature branch is preserved.
Also nudge the executor popover padding: when the prepare-status
section is rendered above the Sprites/Docker info block, the cumulative
top+bottom paddings looked too generous around "Sprites sandbox" /
"Local Docker". Tighten both sections vertically without affecting the
horizontal rhythm or the bottom action row.
* feat(web): show "Resuming session" feedback in popover during reconnect
The bottom Resume button on the chat panel was leaving users staring at
nothing for the 5-30s reconnect window: prepare-progress events are
intentionally suppressed on resume (the workspace is already there), so
the popover sat in the idle phase even while session.state was STARTING.
Synthesize a "resuming" phase from session state when no prepare events
are flowing, rendered as a spinner row in the popover with copy
"Resuming session — Reconnecting to the existing environment…". The
button icon also flips to the spinner so the toolbar advertises the
in-flight work without the user having to open the popover.
Extract the prepare-status reducer + section component into their own
file so the main popover module stays under the 600-line ESLint cap.
* fix(codex): align with the standard SessionDirTemplate pattern
Codex was the only agent that bind-mounted its host home into the
container via an explicit `Mounts: [{home}/.codex → /root/.codex]`
entry, while every other ACP agent uses `SessionConfig.SessionDirTemplate`
+ `SessionDirTarget`. Move codex onto the same surface so the
host-state isolation work in the next commit covers it uniformly.
This change alone does not stop the host-state leak — the bind mount
still resolves to the host's ~/.codex for now — but it removes the
last agent-specific branch in the runtime config so the docker
container manager can apply one rule for all agents.
* fix(docker): isolate per-container agent state from the host home
Every ACP agent's session dir was bind-mounted from ~/.<agent> on the
host into /root/.<agent> in the container. That exposed host state DBs,
session caches, and lock files to the container — codex's rollout DB
caches absolute /Users/... paths, which don't resolve inside Linux and
the agent silently loses prior thread context on resume.
Replace the host bind mount with a kandev-managed per-container session
dir under <kandev-home>/agent-sessions/<instance_id>/. Before the
container starts, selectively copy the auth-relevant files (the same
files Sprites already copies via RemoteAuth.SourceFiles) into that
dir; everything else — state DBs, sessions/, lock files — stays on the
host and the agent rebuilds them inside the container with
container-local Linux paths.
The change is agent-agnostic: any agent that has SessionDirTemplate
+ SessionDirTarget gets the new behavior automatically. Cleanup runs
on destructive stops only (task/session deleted/archived), mirroring
the Sprites preserve-on-stop rule so plain stops let resume re-attach
to the same agent state.
* fix(executor): make the kandev feature-branch checkout an invariant postlude
Profiles created in the UI snapshot the *then-current* DefaultPrepareScript
into their stored prepare_script field. Updates to the default — like
shipping the worktree-branch checkout block — never reach those older
profiles, so users keep landing the agent on the chosen base branch
(main) even though the env row carries a feature branch name.
Append a kandev-managed postlude to whatever prepare_script ran (custom
or default), wrapped in a subshell + `|| true` so it's idempotent and
never blocks agentctl from starting. The postlude prefers the remote tip
when origin already has the branch (resume after a destroyed sandbox),
falls back to checking out the existing local branch, and finally
creates one off the current HEAD.
Sprites needed the WorktreeProvider in resolvePrepareScript so the
postlude's `{{worktree.branch}}` actually expands; Docker already had
it.
The Docker e2e gains a regression case that explicitly stores an older
default-style script (clone, no checkout) and asserts the agent still
ends up on the feature branch — locking the contract in regardless of
which default the user happens to have stored.
* fix(web): tighten executor popover padding under the env header
The Sprites/Docker info block under the Environment-ready row had too
much vertical breathing room — the header-to-fields gap and the
inter-row spacing piled up and the popover felt airy. Trim the section
to match the snug rhythm of the rest of the popover (px-3 py-2,
1.5-unit gap before the field list, 1-unit gap between rows). Also
unify the Environment-ready row from p-3 to px-3 py-2 so the dividers
between sections sit at the same vertical cadence.
* fix(web): tighten executor popover bottom padding too
The previous tighten only fixed the top half — pb under the field rows
plus the Reset-row's full p-2 still left a generous gap below the last
"Sprite ..." line. Same fix applies whether the popover is showing
Docker or Sprites.
EnvironmentInfo: pt-2 pb-1.5 (was py-2) so the bottom field hugs the
divider, and the Reset-row goes from p-2 to px-2 py-1.5 so the button
sits closer to the text above.
* fix(web): drop the redundant Environment-ready row + tighten popover top
The READY badge next to the executor name already conveys ready-state;
the dedicated "Environment ready · 12s" row above the executor info
just added vertical noise. Skip rendering for the ready phase (other
phases that surface non-redundant info — preparing, resuming, failed —
still render). Drop top padding on the executor info section now that
nothing sits above it on the happy path.
* fix(web): move executor info to the left top bar, hover-to-open, no border
The executor section is informational, not a button — readiness state +
sandbox/container/branch fields. Move it to the left top bar (next to
the integrations menu) where it sits alongside the other contextual
info, drop the outline-button styling, and switch from click-to-open to
hover-to-open via HoverCard with 150ms open / 250ms close so the cursor
can travel to the destructive Reset button without flickering.
The trigger is a borderless icon-with-status-dot span; tabIndex keeps
it keyboard-accessible. Reset still works — it opens the existing
confirmation dialog independently of the hover card.
Tests: pointerEnter instead of click for the hover-to-open path.
* refactor(cleanup): drop dead code, dedupe predicates, unify token injection
Tier 1 of the post-review cleanup. Each item is independently safe; bundling
because they're all small, mechanical, and lint-passing.
- fix(clone): unify injectTokenIntoURL across Docker and Sprites — Sprites
used to ignore GH_TOKEN entirely (a user with only GH_TOKEN set got 401
on Sprites clone but worked on Docker). Promote one
`injectGitHubTokenIntoCloneURL` helper, accept both env vars
(GITHUB_TOKEN wins, matching Docker), and use the documented
`https://x-access-token:T@github.com/` form on both surfaces.
- fix(executor-popover): swap the <span tabIndex role="button"> trigger
for <Button variant="ghost"> so Enter/Space activation, focus rings on
Safari, and disabled semantics come for free.
- test(docker): rewrite TestDockerStopInstance_PreservesSessionDirOnPlainStop
to drive the real DockerExecutor.StopInstance path with ContainerID=""
(skips the docker daemon, exercises the kandev-side cleanup branch).
Previous version only iterated shouldRunExecutorCleanup strings — would
pass even if StopInstance always cleaned up. Now table-driven over
preserve+destructive reasons with on-disk asserts.
- refactor(orchestrator): drop dead `_ = arg` parameters from persist*
helpers (persistResumeState, persistModelSwitchState, persistLaunchState).
Brings signatures from 9→6 args, under the funlen lint cap.
- refactor(orchestrator): collapse byte-identical isContainerizedExecutor
≡ isCloneBasedExecutorType predicate into one.
- refactor(lifecycle): drop dead getMetadataBool, FileUploader.RunCommand,
expandMountSource {home} arm (no production agent ships {home} in
Mounts; codex_acp_test asserts it), dockerTaskBranch one-line wrapper.
* refactor(scripts): postlude owns worktree-branch checkout
Strip the inline `git checkout -B` block from the Docker and Sprites
default prepare scripts. The kandev-managed postlude
(KandevBranchCheckoutPostlude) is appended unconditionally after every
user prepare script, so keeping the same logic inline made the default
and the postlude two sources of truth that could drift.
* refactor(lifecycle): drop dead instance_port persistence entry
`instance_port` is only used as a zap log field — it's never written to
the executor metadata map, so carrying it across resume in
`persistentMetadataKeys` was a no-op.
* refactor(lifecycle): drop legacy {home} session-dir fallback
ExpandSessionDir kept a `{home}/<dotdir>` fallback for the case where
kandevHomeDir was unresolved. In production, ResolvedHomeDir() always
returns a non-empty path, so the fallback only ever fired in tests
that hadn't been updated to pass an instance ID. Returning the empty
string in that case makes container.go skip the bind mount — the
correct behaviour, since without a kandev home there's no session dir
to mount.
* refactor(orchestrator): collapse env-reuse helpers into reuseExistingEnvironment
The two env-reuse helpers (`applyExistingEnvironment` and
`applyExistingEnvironmentRuntimeMetadata`) were always called together
and shared the same intent: carry the previous TaskEnvironment forward
into the launch request. Merge them into a single
`reuseExistingEnvironment(ctx, req, env)` so the executor-type gate
runs once for both env-level fields and runtime metadata, and callers
have a single entry point.
Document the layered metadata rule (running wins for container_id;
env wins for sprite_name and other stable IDs) inline so the next
reader doesn't have to reverse-engineer it from two functions.
* refactor(web): lift prepare-summary reducer + isFallbackNoticeStep into lib/prepare
Move the popover's prepare-summary reducer (and the
`isFallbackNoticeStep` helper duplicated word-for-word in
`prepare-progress.tsx`) into `apps/web/lib/prepare/summarize.ts`. The
hook moves to `hooks/domains/session/use-prepare-summary.ts` to match
the project's domain-organized hook layout.
The popover's PrepareSummary type and the chat panel's EffectiveStatus
serve different UIs, so the panel keeps its own `deriveStatus` — only
the literal duplicate (`isFallbackNoticeStep`) is shared.
* refactor(web): split executor-settings-button by concern
Slice the 471-line button file into three modules with single
concerns: the popover shell stays in `executor-settings-button.tsx`
(168 lines, just trigger + popover layout + icon/status dot),
`executor-environment-info.tsx` owns the env detail rows + status
badge, and `hooks/domains/session/use-task-environment.ts` owns the
fetch/poll/reset lifecycle alongside other session hooks.
* refactor(e2e): extract docker + session helpers from docker-launch.spec
Pull the inline docker CLI helpers (`dockerInspectExists`, `dockerStop`,
`dockerCurrentBranch`, etc.) and the generic session-state pollers
(`waitForLatestSessionDone`, `waitForSessionDone`,
`waitForSessionEnvironment`) into `e2e/helpers/docker.ts` and
`e2e/helpers/session.ts`. The spec drops from 763 to 647 lines and the
helpers can be reused by future docker / session tests.
* test(scripts): execute postlude under bash -e against a real git repo
Pair the static-content assertion with a behaviour test that runs the
postlude as a real shell script. Three scenarios exercise the three
branches inside the snippet: remote tip exists (track origin), no
remote (fresh local branch), and already on the target branch
(idempotency). This catches regressions where the snippet still parses
as bash but no longer does the right thing — wrong refspec, missing
-B, swallowed exit code, etc.
* test(sprites): integration check for stop preserve + resume reconnect path
Each piece of the resume-fast path is unit-tested separately
(StopInstance keeps the token cache, spritesShouldReconnect reads
sprite_name, the reconnect plan has 3 steps). This new test wires them
together end to end so a future change that accidentally strips
sprite_name on stop or forgets to look at metadata on launch fails
loudly here, not at runtime.
* test(sprites): assert plan ordering instead of absolute indexes
Replace `plan.index(spriteStepWaitHealthy) == 1` with an
`assertPlanOrder` helper that checks "X runs before Y". The contract
the test guards is the order of create-sprite → wait-healthy →
agent-instance, not their absolute slot numbers — adding a new step
shouldn't break the test if the order is preserved.
* test(web): hoist test constants above vi.mock factories
Move SESSION_ID and friends above the vi.mock factories so the mock
closures don't depend on vitest hoisting magic capturing TDZ-undefined
locals. The previous layout worked because factories aren't invoked
until the module under test is imported (after const initialization),
but a refactor that imports earlier would break the closure silently.
* fix(seeder): contain localFileUploader writes inside the session root
CodeQL flagged go/path-injection on the localFileUploader.WriteFile
sink: instanceID flows into the write path, and a malformed value
would let writes escape the kandev session root and clobber arbitrary
host files. Bind the uploader to its session root and reject any path
that filepath.Rel resolves to a "../" escape. Production callers
always pass an internally generated instanceID; the check is defense
in depth against a future caller plumbing in user input.
* refactor(docker): split reconnectToContainer + cover reconnect helpers with unit tests
Address recurring review feedback on the docker reconnect path:
- reconnectToContainer was 98 lines (over the 80-line repo limit). Split
into ensureContainerRunning (inspect + start + re-inspect) and
bringupAgentctl (health-check, find-or-create instance, transparent
re-handshake). The orchestration body now reads top-down at 49 lines.
- resolveDockerEndpoint and waitForAgentctlHealth grew real branches in
this PR (host-port lookup with container-IP fallback; 240×500ms
retry loop) but had no direct tests. Introduce narrow hostPortLookup
/ healthChecker interfaces, satisfied by *docker.Client and
*agentctl.ControlClient, so both functions are now driven by stubs
that exercise happy-path, fallback, retry-until-healthy, exhaustion,
and context-cancellation branches.
- Add direct unit tests for the docker package's pure helpers
(normalizeDockerHostIP, buildDockerPortBindings) and for
dockerAgentctlPortBindings — a regression in the published port
range or host IP would silently break container reconnect.
* test(handlers): cover ensureShellExecution stale-execution recovery
The recovery branch (Failed execution → CleanupStaleExecutionBySessionID
→ relaunch) is the load-bearing piece of shell session resilience and
had no direct test. Extract the body into a free function that takes a
narrow shellLifecycle interface so the four branches are testable
without standing up a real lifecycle.Manager:
- happy path (running execution, no cleanup)
- stale recovery (Failed → cleanup → fresh)
- initial GetOrEnsureExecution failure
- cleanup failure must NOT trigger a re-launch loop
- relaunch failure surfaces a clear "recover execution" error
* fix(backend): clear lint blockers from CI lint job
CI's only-new-issues lint surfaced three real items left over from
recent additions:
- container.go bootstrap snippet had two `fi` shell tokens flagged by
dupword. Tagged with the same nolint directive used by the
KandevBranchCheckoutPostlude (two distinct shell blocks closing).
- DockerExecutor.CreateInstance was 89 lines (over funlen 80). Split
the body into helpers: reportCreateInstanceProgress (deferred
progress reporter), tryReconnect (gates the resume path),
seedSessionDir (per-container auth seed), buildContainerLaunchConfig,
and buildCreatedInstance. CreateInstance is now a 30-line top-down
orchestrator.
- executor_execute.go was 887 effective lines (over revive 800). Move
reuseExistingEnvironment + its helpers (latestExecutorRunningForEnv,
executorRunningMatchesEnvironment, applyExecutorRunningMetadata,
ensureLaunchMetadata) into a sibling executor_environment_reuse.go.
* fix(web): only block submit during real env prepare, not every STARTING
The "agent still being set up" submit gate was triggering on every
session that briefly transitions through STARTING — including local
quick-chat, which has no prepare phase and has its agent live within
~100ms. The eager-init quick-chat e2e tests (chat/quick-chat.spec.ts
and chat/message-queue.spec.ts) wait for the editor to become
editable, then press Cmd+Enter. The race made wrappedSubmit a no-op
exactly when the test was firing — message dropped, agent never saw
it, downstream assertions failed.
Split the merged isStarting flag in useSessionState: keep the broad
flag (still used for the workspace-prep loading UI), and expose
isPreparingEnvironment as a separate signal that's only true while
prepareProgress.status === "preparing". Thread it through chat-input-
area → chat-input-container → useChatInputContainer and gate the
submit-disabled tooltip on it instead of the broad isStarting.
Net: Docker / Sprites bootstrap still surfaces the "agent still being
set up" message and disables submit; quick-chat / local sessions stop
silently dropping in-flight messages.
* fix(web): keep editor disabled during STARTING so quick-chat sends don't race
My previous fix made submit no longer block on isStarting, with the
goal of keeping local quick-chat sessions from silently dropping
messages. The trace from CI showed it traded one failure for another:
the editor became contenteditable while the session was still
STARTING, the e2e suite pressed Cmd+Enter immediately, and the
backend rejected with "Failed to send message to agent" because the
agent process wasn't ready yet.
Restore the original semantics — STARTING blocks both editor and
submit — but keep `isPreparingEnvironment` as the narrower trigger for
the "agent still being set up" tooltip. Net behaviour:
- STARTING transition: editor disabled, no tooltip (signal is the
disabled state itself)
- container/sandbox prepare: editor disabled + tooltip explaining why
- quick-chat sessions: editor stays disabled until RUNNING, no race
* docs(web): correct isPreparingEnvironment JSDoc after STARTING re-block
The
|
||
|
Carlos Florêncio
|
00fed4661e | fix(orchestrator): resolve subtask base_branch correctly across repos (#870) |