0f8da4ff28028f0aa36461e344f3dd07d1662a6b
130 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Carlos Florêncio
|
0f8da4ff28 | fix: unblock release desktop and universal builds (#1522) | ||
|
Carlos Florêncio
|
14a5fb2c7f | fix: support unsigned desktop releases (#1520) | ||
|
Carlos Florêncio
|
211e738ec0 | fix: reuse worktrees for same-task handoffs (#1510) | ||
|
Carlos Florêncio
|
9484bc33d9 | fix: prime task create last-used cache before dialog effects (#1509) | ||
|
Carlos Florêncio
|
5252a55199 | fix: improve pr ci automation reliability (#1508) | ||
|
Carlos Florêncio
|
49c482bd4d | fix: prevent leaked agent processes on shutdown (#1498) | ||
|
Nicolas Lenepveu
|
5021066d78 | feat: bind repository to Linear/Jira/Sentry issue watchers (#1491) | ||
|
Carlos Florêncio
|
81ef4fb5e5 | fix: clean office workspace data on delete (#1486) | ||
|
Carlos Florêncio
|
a28c4226de | fix: make schema migrations replayable (#1489) | ||
|
Carlos Florêncio
|
7e92f8da7a | feat: add tauri desktop app (#1478) | ||
|
Carlos Florêncio
|
4d19fb32f1 | feat: link tasks to github references (#1471) | ||
|
Carlos Florêncio
|
6126edb203 | feat: add subtask completion trigger (#1473) | ||
|
Carlos Florêncio
|
e91975be17 | feat: improve ci auto-fix prompt control (#1474) | ||
|
Carlos Florêncio
|
991c7817ad | fix: stop task runtimes from leaking (#1465) | ||
|
Carlos Florêncio
|
4f9407ed54 | fix: show queued CI auto-fix prompts in order (#1463) | ||
|
Carlos Florêncio
|
c763ece50a | docs: refresh harness guidance (#1458) | ||
|
Carlos Florêncio
|
a9ed563749 | fix: stop run-mode automation agents (#1459) | ||
|
Carlos Florêncio
|
da45983c4c | feat: add native kandev launcher (#1391) | ||
|
Carlos Florêncio
|
f4a906884f | fix: stop CI auto-fix on non-actionable feedback (#1452) | ||
|
Carlos Florêncio
|
7ada858f17 | feat: add CI PR automation controls (#1446) | ||
|
Carlos Florêncio
|
aee1846cd5 | feat: remove nextjs production runtime (#1389) | ||
|
Carlos Florêncio
|
6a7bf92893 | docs: refresh feature documentation (#1424) | ||
|
Carlos Florêncio
|
d8436dfd8f | feat: improve passthrough composer and session recovery (#1411) | ||
|
Carlos Florêncio
|
fcd2cc00e9 | fix: open markdown file links with line suffixes (#1404) | ||
|
Carlos Florêncio
|
f9608ba52d | fix: expose kandev mcp tools to pi (#1401) | ||
|
Carlos Florêncio
|
e4b39238fa | docs: update spec-driven planning workflow (#1390) | ||
|
Carlos Florêncio
|
fd421848fb | feat: add feature toggles settings (#1372) | ||
|
Carlos Florêncio
|
3a3fca7f0e | feat: add resource metrics topbar (#1373) | ||
|
Carlos Florêncio
|
d6cf4a1acc | chore: add provider-agnostic bootstrap (#1377) | ||
|
Carlos Florêncio
|
010948767a | feat: add file-backed prompt attachments (#1367) | ||
|
Carlos Florêncio
|
cf3469b4d1 | feat: allow read-only absolute file paths (#1371) | ||
|
Carlos Florêncio
|
326297c3e7 | fix(agent): avoid oversized env spawn failures (#1344) | ||
|
Carlos Florêncio
|
fc4bf8a40c | feat: add checkout service make targets (#1311) | ||
|
Carlos Florêncio
|
637123f84f | feat: allow remote external mcp access (#1307) | ||
|
Carlos Florêncio
|
1484ae7d28 | fix: expose user agent bins to system service (#1292) | ||
|
João Salavisa
|
97c1f689e4 |
feat(workflow): explicit step-completion signal for auto-advance (#1276)
* feat(workflow): explicit step-completion signal for auto-advance Adds `step_complete_kandev` MCP tool + `WorkflowStep.auto_advance_requires_signal` column so workflow steps can gate their `on_turn_complete` transitions on an agent-emitted completion signal instead of inferring "done" from any turn-end. See docs/decisions/0015-explicit-completion-signal-for-auto-advance.md for the motivation, alternatives considered, and trade-offs. * fix(workflow): address PR review feedback for explicit completion signal - Dedup `LoadPendingStepSignal` into `internal/task/models` so the MCP handler (write site) and the orchestrator (read site) share one decoder. - Gate the out-of-band subscriber on `WorkflowStep.AutoAdvanceRequiresSignal` so `step_complete_kandev` is not an unintended manual-advance path for signalless steps. - Fail closed on step-load errors in `processOnTurnCompleteViaEngine`: a loader hiccup must block the transition rather than silently bypass the gate. - Reject `step_complete_kandev` for terminal sessions (Completed / Failed / Cancelled) so the agent gets a clear error instead of `accepted: true` followed by a no-op. - Surface bus-publish failures back to the agent instead of swallowing them — the idempotency guard makes the retry safe and prevents the session from getting stuck. - New `TestOnStepCompletionSignaled` covers the subscriber paths (RUNNING / WAITING+match / stale step / not signal-gated). - ADR doc fixes: payload field list now matches the wire shape, writer ownership is documented, the unsubstantiated 250ms-debounce claim is removed, and the late-signal re-close race is recorded as a deferred open question. - Drop the new toggle's HelpTip in favour of an inline description — the shadcn tooltip portal was intercepting clicks on the agent-profile dropdown options in the workflow settings E2E. * fix(workflow): round-2 review fixes for explicit completion signal - Split `handleStepComplete` into `resolveStepCompleteTarget` + `publishStepCompletionEvent` helpers so the entrypoint clears funlen (88 → under 80 lines) and the two halves are independently testable. - Classify session/task load errors instead of returning `NotFound` for every failure: use the exported `taskrepo.ErrTaskNotFound` sentinel for task lookups, substring-match for the session repo (no sentinel today), and surface anything else as `InternalError` with the raw error logged but not leaked to the agent — so a transient DB hiccup triggers a retry rather than session abandonment. - Pin `step_complete_kandev` explicitly in the mode-specific tool-count tests: `Contains` in `ModeTask`, `NotContains` in `ModeConfig` and `ModeOffice`, so accidental dereg or cross-mode registration is caught by the diff instead of by a still-equal count. * fix(workflow): round-3 review fixes + defer pipeline-editor toggle - Defer the "Wait for agent completion signal" sub-toggle to a follow-up PR. Adding it to the step config card shifts the layout enough that the agent profile select's Radix popper opens into a sibling tooltip's hover path, breaking workflow-agent-profile.spec.ts on shard 7. The backend column ships in this PR — workflows can opt in via the YAML import or the MCP `update_workflow_step_kandev` tool — and the UI affordance gets its own iteration where the test interference can be investigated without blocking the rest of the change. - Re-publish the bus event on the `already_signaled` short-circuit when the session is WAITING_FOR_INPUT. Without this, an agent's retry after the first call's publish failed would short-circuit on the persisted bag, never fire the subscriber, and leave the session stuck. - handleStepComplete docstring: drop the non-existent feature-flag claim and the "audit entry" wording — gating is purely `WorkflowStep.AutoAdvanceRequiresSignal` and the bag is cleared on the next turn start with no separate audit trail. - `step_complete_kandev` tool description: same fix — clarify that the call succeeds for non-gated steps but the signal is discarded on the next turn start (no audit history to query later). * test(workflow): cover handleStepComplete + reword defer-comment - Add `TestHandleStepComplete_*` (6 subtests, all passing) covering the uncovered handler paths flagged by Claude/cubic on round 3: missing-fields validation, session-does-not-belong-to-task ownership guard, terminal-session rejection (Completed/Failed/Cancelled), first-call happy path (bag persisted + bus payload shape pinned), dedup while RUNNING (no re-publish), and dedup while WAITING_FOR_INPUT (the retry-after-publish-failure path — must re-publish so the subscriber fires). - Reword the deferred-toggle comment to drop the YAML-export claim: the workflow export model doesn't yet serialize `auto_advance_requires_signal`, so MCP `update_workflow_step_kandev` is the only configuration path that round-trips today. Note the follow-up needs to extend the export schema alongside the UI toggle. * test(workflow): pin signaled_at + drop redundant GetTaskSession on bag-clear - Assert `signaled_at` is present and parses as RFC3339Nano in `TestHandleStepComplete_FirstCallAccepted`. The field is part of the handler's documented response contract; pinning it catches accidental drop/rename regressions. - Add `clearPendingStepSignalByID(ctx, sessionID)` as a thin DB-only variant of `clearPendingStepSignal` and use it from `executeStepTransition`, where the session struct isn't read after the clear. Avoids an extra `GetTaskSession` round-trip per transition. `clearPendingStepSignal(session)` still exists for the in-memory-mutation case (subscriber's stale-step path). * fix(workflow): subscriber stale-clear guard + ADR/comment drift fixes - onStepCompletionSignaled's stale-step path now reads the bag and only clears it when its current StepID matches the (stale) event's stepID. Previously the unconditional clear could destroy a valid signal that the new step's agent had just written when sessions are reused across steps without auto_start_agent. (Greptile P1.) - Drop the "feature flag" phrase from WorkflowStep.AutoAdvanceRequiresSignal docstring — leftover from an earlier draft. The gating is purely the boolean field; the global flag was removed two rounds ago. - Merge the two adjacent `if triggerOnEnter` blocks in executeStepTransition so the full triggerOnEnter=true path reads top- to-bottom in one place. (CodeRabbit + cubic.) - ADR 0015 Mechanism section: re-label the SessionStepHistory.Metadata audit annotation, the `step_complete_signaled` system message, and the expvar telemetry counters as deferred. They're planned shapes, not current behaviour — calling them "Audit" / "Chat visibility" / "Telemetry" in present tense was misleading for future readers. * fix(workflow): drop subscriber complexity + reload session on stale-clear - Extract `parseStepCompletionEvent` and `stepIsSignalGated` helpers from `onStepCompletionSignaled`. Cyclomatic complexity goes 17 → under 15 (cyclop on new code, CI lint regression after the round-6 stale-clear guard). - The stale-step branch now re-loads the session from the DB before deciding whether to clear the bag. The previous round-6 fix checked the local `session.Metadata` snapshot, which can be older than a concurrent write from the new step's agent — that check could see a stale signal that had since been overwritten with a valid one and erase the new bag entry. The narrower load-on-clear is cheap and closes the cubic P1 hole. (The broader race in `processOnTurnCompleteViaEngine` reading session.Metadata is the monotonic-marker open question in ADR 0015; out of scope here.) * test(workflow): pin stale-event guard's preserve-current-bag side + dedup StringFromAny - Add `TestOnStepCompletionSignaled` subtest "stale event → valid bag for CURRENT step is preserved" — fires a stale step-A event while the bag holds a valid step-B signal, asserts the bag still contains StepID=step_b after the subscriber runs. Pins the negative side of the round-6 `existing.StepID == stepID` guard, so a regression that drops or inverts the guard (and would silently leave signal-gated steps stuck) fails the test instead of slipping through. - Promote `pendingSignalString` to exported `models.StringFromAny` and reuse it from the orchestrator subscriber's `parseStepCompletionEvent`. Eliminates the byte-for-byte duplicate helper across the two packages; the bag-shape decoder and the bus- event decoder now share one source of truth for "missing string". --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
Carlos Florêncio
|
7917c95496 | feat(backend): inject MCP servers in CLI passthrough mode per agent (#1158) | ||
|
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> |
||
|
José Almeida
|
041a19f060 |
feat: service-managed UI self-update (#1210)
* feat: add service-managed self-update * chore: add temporary self-update test scripts * chore: simplify temporary script markdown fences * chore: run frontend in temporary self-update scripts * fix: show self-update job progress after apply * chore: add real npm self-update test script * fix: isolate npm self-update prefix * fix: detect npm service installs from bin shim * chore: reset service before real npm update test * chore: expand real npm status diagnostics * fix: preserve service home during self-update * fix(cli): tee self-update helper output to a log file * fix(cli): avoid launchd bootstrap race when reloading the macOS service * fix(backend): run macOS self-update helper once via transient launchd plist * fix(web): show self-update progress and lock the apply button until the update lands * fix(cli): reinstall via the kandev wrapper on Homebrew self-update * chore: remove temp self-update test scripts * fix(web): drop unused apply path from useUpdates hook * fix(backend): snapshot install state once in apply preflight * docs: align self-update docs with transient LaunchAgent flow * chore(release): run cli bundle from apps workspace root * fix(backend): guard against concurrent self-update applies * fix(backend): check scheme and helper paths in self-update apply * fix(backend): drop duplicate binary name from npx manual command * fix(cli): allow --help for service self-update without --intent * fix(cli): harden self-update dir perms and darwin uid resolution * fix(web): set required POST fields last in applyUpdate * docs: use JSON body notation for the apply endpoint * fix(backend): expire self-update guard so a stuck helper cannot wedge apply * refactor(cli): collapse redundant writeSync branch in self-update log |
||
|
Carlos Florêncio
|
86f756e280 | feat(workflow): add set_session_mode action and preserve mode across reset (#1188) | ||
|
Carlos Florêncio
|
dcde3da74c | feat: surface an inline notice when an agent turn produces no output (#1179) | ||
|
Carlos Florêncio
|
e708a76701 | docs: document portable workflow import/export YAML format (#1176) | ||
|
Carlos Florêncio
|
ec289c799b | feat: retry transient provider 529 errors with visible backoff (#1173) | ||
|
João Salavisa
|
2cb3f4a384 |
chore: remove dead non-ACP provider client packages (#1083)
* chore: remove dead non-ACP provider client packages Delete five Go packages under apps/backend/pkg/ that were unreachable from any binary: amp, codex, claudecode, copilot, opencode. These were the client libraries for legacy non-ACP agent implementations removed in |
||
|
José Almeida
|
823117ba48 |
fix: wire MCP config into CLI passthrough (#1078)
* fix: wire MCP config into CLI passthrough * fix: address passthrough MCP review feedback * fix: fail fast when passthrough MCP port is missing * fix: track passthrough MCP config with nil metadata |
||
|
Carlos Florêncio
|
f1dbad1dbd | feat(cli-mode): auto-inject task prompt + dynamic submit sequence + passthrough UX fixes (#923) | ||
|
Carlos Florêncio
|
8a7b7a0b60 | feat(agentctl): azure Repos PR follow-ups after #1066 (#1071) | ||
|
José Almeida
|
777dfdb15b |
feat(ssh): add SSH executor (#927)
* feat(backend): add SSH executor runtime
Run agents on any Linux box reachable over SSH: own VPS, homelab, EC2,
shared dev box. One SSH connection per (host, user, identity) shared
across all sessions on that host; each session gets its own remote
agentctl process, remote port, and SSH-tunneled local port. Mirrors the
local ~/.kandev/tasks/<task-dir>/<repo>/ layout on the remote so
multi-session tasks share a worktree and per-session runtime state lives
under the task dir.
Implements ExecutorBackend + RemoteSessionResumer + RemoteStatusProvider
+ Closeable. Host-key trust is explicit: the test endpoint observes a
fingerprint without pinning; the user trusts it in settings; subsequent
connects reject any mismatch. Auth via ssh-agent or IdentityFile only
(passphrase-protected keys must be loaded into ssh-agent). Parses
~/.ssh/config for HostName/Port/User/IdentityFile/ProxyJump inheritance.
agentctl is content-hash-cached on the remote (~/.kandev/bin/agentctl
plus a .sha256 sidecar) and uploaded only when local and remote differ.
Linux/amd64 only in v1, matching Docker's current state; arm64 is
called out as a cross-cutting follow-up.
See docs/specs/ssh-executor/spec.md.
* feat(backend): add SSH test + sessions HTTP/WS endpoints
POST /api/v1/ssh/test runs the connection probe used by the settings UI:
dials the host with a permissive host-key callback that records (but
does not pin) the fingerprint, probes uname/arch/git, and reports
whether agentctl is already cached on the remote. The UI gates the
"Trust this host" checkbox on success + fingerprint, and Save persists
the trusted fingerprint into the executor config.
GET /api/v1/ssh/executors/:id/sessions returns the active SSH-runtime
sessions backing the "Active sessions" card on the executor settings
page, mirroring the SpritesInstancesCard / DockerContainersCard shape
(task, session, user@host, remote port, local forward port, uptime).
Both routes also exposed over WS via ssh.test / ssh.sessions.list.
* feat(web): add SSH executor settings UI
SSHConnectionCard: connection form (name, host_alias / host / port /
user, identity source = ssh-agent | file with optional file path,
optional ProxyJump) with a Test Connection button that runs the backend
probe, surfaces the observed host fingerprint, and only enables Save
when the user explicitly ticks "Trust this host". Editing the host /
port / user / identity fields resets both the test result and the trust
checkbox. On save, if the executor already has running sessions, a
confirm modal warns that those keep going on the old host and only new
sessions will use the new config.
SSHSessionsCard: 90s-polled table of active sessions for the executor,
mirroring SpritesInstancesCard / DockerContainersCard. Columns: task,
session, user@host, remote agentctl port, local forward port, uptime,
status badge. Backed by GET /api/v1/ssh/executors/:id/sessions.
Also registers the ssh entry in executor-icons.ts (label "SSH",
IconTerminal2) so it surfaces consistently in the rest of the settings
chrome.
* docs: add SSH executor v1 spec
Captures the design behind the new SSH executor: why ("run on a Linux
box I already own" between local and Sprites), the remote layout that
mirrors local ~/.kandev/tasks/, per-session agentctl + forward over a
shared per-host SSH connection, explicit-trust host-key flow, ~/.ssh/config
inheritance, amd64 + agentctl content-hash cache, recovery via persisted
ExecutorRunning metadata, settings UI (Connection + Sessions cards), and
the edit-with-live-sessions warning UX. Out-of-scope items (arm64,
macOS, password auth, chained ProxyJump, working-tree push, etc.) are
called out explicitly with the follow-ups they unblock.
* chore(e2e): rename docker project to containers + add e2e README
The Docker e2e Playwright project was the natural home for any test
that needs a real Docker daemon on the host — but its name implied it
was only about the Docker executor. SSH e2e tests (incoming in
follow-up commits) belong here too, since they use a Dockerised sshd
container as the SSH executor's remote target.
- Rename project: docker -> containers, widen testMatch to also include
tests/ssh/*.spec.ts.
- Rename env flag: KANDEV_E2E_DOCKER -> KANDEV_E2E_CONTAINERS, with the
old name kept as a deprecated alias for one release so local scripts
and CI configs don't break overnight.
- Rename CI job: e2e-docker -> e2e-containers, switch its blob report
name and the merge-job dependency.
- Add apps/web/e2e/README.md explaining the project layout, what the
containers project covers (Docker executor + SSH executor), why it
was renamed, and how to run it locally.
- Add docs/specs/ssh-executor/e2e-plan.md tracking the SSH e2e
coverage we'll implement on top of this rename (real sshd container,
no mocks, 17 spec files covering ~75 cases). Includes a "Deferred"
section that flags arm64 as something to revisit when
agentctl-linux-arm64 lands in the build pipeline.
* feat(web): add data-testid attributes to SSH settings UI
Sweep stable test ids across the SSH connection and sessions cards so
the incoming Playwright suite can hold steady against refactors. Splits
the formerly monolithic ssh-settings.tsx into three files to stay
within the per-file/per-function size limits:
- ssh-connection-card.tsx (form + test-then-trust flow)
- ssh-sessions-card.tsx (active-sessions table)
- ssh-settings.tsx (barrel re-export so existing imports keep working)
Test ids cover: card roots, every input, identity-source select +
options, test/save buttons, connection badge (with data-status), error
text, test-result panel (with data-success), per-step rows (with
data-success), observed/pinned fingerprint, fingerprint-change
warning, trust checkbox, sessions table + per-row cells, empty state,
refresh button, session status badge.
* feat(e2e): scaffold the SSH executor test harness
Lands the building blocks the SSH specs will compose on top of. Every
piece talks to a real sshd container — no mocks at any layer.
- fixtures/ssh-image.ts: builds kandev-sshd:e2e once per machine (Alpine
+ openssh-server + openssh-sftp-server + git + bash + sudo +
iproute2 + iptables, pre-bakes mock-agent at /usr/local/bin/mock-agent,
generates a fresh host key on entrypoint, reads the worker public key
out of a bind-mounted /authorized_keys). The agentctl binary is
deliberately NOT pre-baked so tests exercise the SFTP upload + sha256
cache path the production code uses.
- helpers/ssh.ts: per-worker sshd lifecycle (startSSHServer /
stopSSHServer), keypair generation, host-key fingerprint scanning,
fault injection (regenerateHostKey for host-key-rotation specs,
dropTrafficToPort22 / restoreTraffic for connection-drop tests,
killRemotePid for "agentctl died" recovery), and remote inspection
helpers (readRemoteFile / remotePathExists / listRemoteDir /
countSshdConnections).
- helpers/ssh-bastion.ts: two-container ProxyJump setup. Bastion is on
a private Docker network and has a published port; target is on the
same network with NO host port — any successful connection MUST have
traversed the bastion, which makes the proxy-jump assertion honest.
- fixtures/ssh-test-base.ts: worker-scoped fixture mirroring
docker-test-base. Skips when Docker isn't reachable. Spins up the
sshd container, generates keys, scans the fingerprint, then seeds a
workspace + workflow + bare repo + mock-agent profile + an SSH
executor with the fingerprint already trusted. Specs that need to
exercise the test-then-trust UI gate create their own executor on
top of the same target.
- pages/SSHSettingsPage.ts: Page Object covering the connection form,
test/save buttons, trust checkbox, observed/pinned fingerprint, per
step badges, and the sessions table.
- helpers/api-client.ts: new createSSHExecutor / updateExecutor /
getExecutor / testSSHConnection / listSSHSessions methods plus
matching DTO types.
* test(e2e): add SSH executor Playwright suite (17 specs, ~75 cases)
Every spec drives the real sshd container set up by the ssh-test-base
fixture; no mocks. Specs are grouped by concern (file per group),
mirroring the docker/ directory style. Each one maps 1:1 to a row in
docs/specs/ssh-executor/e2e-plan.md.
UI surface:
- connection-form.spec.ts — field rendering, save/test gating,
identity-source toggle, default port.
- test-result.spec.ts — successful + failed Test Connection paths,
per-step badges, fingerprint surfacing, cached/will-upload copy.
- trust-gate.spec.ts — save disabled until trust ticked, edits reset
result + trust, fingerprint-change warning when pinned != observed.
- sessions-card.spec.ts — empty state, row rendering, manual refresh,
status badges, truncation, polling pickup.
- persistence.spec.ts — fingerprint persists across reload, new form
starts blank, executor list label, partial state doesn't leak.
API contract:
- test-endpoint.spec.ts — POST /api/v1/ssh/test step ordering, invalid
body, host-required, handshake short-circuit, cached/uploaded states.
- sessions-endpoint.spec.ts — GET /api/v1/ssh/executors/:id/sessions
filters to ssh-runtime rows, populates metadata, monotonic uptime.
- executor-crud.spec.ts — type=ssh persists fingerprint, profile
workdir round-trip, edit-with-running-sessions keeps snapshots.
Real-SSH integration:
- launch-task.spec.ts — full smoke: agentctl upload + sha256 cache hit
on second launch, task/session dir layout, stop cleans session dir.
- recovery.spec.ts — backend restart reconnects, dead pid surfaces
failure, persisted metadata keys present.
- auth-methods.spec.ts — key + valid, missing key, passphrase-protected
rejection, agent without SSH_AUTH_SOCK, wrong-key auth failure.
- concurrency.spec.ts — two sessions share task dir distinct ports, two
tasks distinct dirs, traffic drop + restore exercises eviction +
reconnect.
- hostkey-rotation.spec.ts — regenerate host key surfaces new
fingerprint, stale pinned fingerprint blocks launch verbatim, re-trust
restores function.
- config-inheritance.spec.ts — ~/.ssh/config HostName / Port /
IdentityFile / ProxyJump inheritance with form-overrides-config rules.
- workdir-per-profile.spec.ts — per-profile ssh_workdir_root routes to
separate remote workdir trees; default falls back to ~/.kandev.
- proxy-jump.spec.ts — 2-container bastion topology asserts target is
unreachable directly, reachable via ProxyJump, chained ProxyJump fails.
- error-surfacing.spec.ts — TCP refused, auth failed, bastion unreachable
surface verbatim; backend 400 stays a 400 not a synthesised failure.
Also adds an AGENTS.md (CLAUDE.md symlink) line pointing at the
e2e/README.md so future agents discover the containers project +
KANDEV_E2E_CONTAINERS env var without grepping playwright.config.ts.
* fix(e2e): unlock sshd account, derive fingerprint from kandev probe
Three bugs surfaced by actually running the SSH e2e suite
(KANDEV_E2E_CONTAINERS=1 against the containers project):
1. Alpine's `adduser -D kandev` leaves the account password-locked
(`!` in /etc/shadow). sshd then refuses every auth method for that
user — including pubkey — with "User kandev not allowed because
account is locked". Fix: `usermod -p '*' kandev` in the Dockerfile
so the account is open for key-based login but still has no valid
password. Also adds the `shadow` apk package for `usermod`.
2. ssh-keyscan reports the ed25519 host fingerprint, but Go's ssh
client may negotiate a different host key type when sshd offers
multiple — meaning the fingerprint the fixture pre-scanned didn't
match the one kandev observed at handshake time. Fix: the fixture
now hits /api/v1/ssh/test with a permissive callback right after
the container is up and uses that fingerprint as the pinned value.
Same code path real users hit; eliminates the negotiation skew by
construction.
3. ssh.ts had a CPU-spin sleep helper and an nc-dependent TCP probe,
both fragile. Replaced with a proper sleep subprocess and a bash
/dev/tcp probe, plus retry-with-stderr on the fingerprint scan so
failures are diagnosable.
Also tightens test-endpoint.spec.ts F4: the test was using
identity_source=agent to "fail fast", but that fails inside
buildAuthMethods before the handshake step ever runs. Switched to
identity_source=file against a refused port so the test exercises
the actual TCP+handshake failure path.
After these fixes 4/6 test-endpoint cases pass against a real sshd.
* fix(ssh): make the end-to-end launch path actually work
A pass of issues uncovered by running the SSH e2e suite locally
(KANDEV_E2E_CONTAINERS=1 against the real sshd container). After this
commit all 15 non-launch SSH e2e tests pass (UI form validation,
test-connection contract, auth-methods, error-surfacing, sessions-card
empty state) and the launch path runs through prepare -> upload ->
agentctl -> instance -> forward, with WebSocket streams sometimes
disconnecting partway through (tracked below for follow-up).
Image changes (apps/web/e2e/fixtures/ssh-image.ts):
- Install curl so the SSH executor can POST to the in-container
agentctl control server for instance creation.
- Install gcompat + libstdc++ so the dynamically-linked glibc
agentctl binary can exec on musl-libc Alpine. Without it the
kernel returns ENOENT for /lib64/ld-linux-x86-64.so.2.
- Install shadow + 'usermod -p *' kandev so sshd doesn't refuse
pubkey auth for the otherwise-locked user.
- Drop in 'AllowTcpForwarding yes' and 'MaxSessions 100' via
/etc/ssh/sshd_config.d/. Alpine's defaults are 'no' and 10, both
of which crater the SSH executor's port-forward path.
- chown /home/kandev/.kandev to kandev on entrypoint so a host
bind-mount of that path (used in this commit for log persistence)
doesn't make agentctl unable to write its log file.
Image helper (apps/web/e2e/helpers/ssh.ts):
- Bind-mount the host mock-agent-linux-amd64 into the container at
the same absolute path. kandev's orchestrator builds the agent
command from KANDEV_MOCK_AGENT_LINUX_BINARY (a host path), so the
remote needs the binary at that exact path; bake-into-image isn't
enough because the absolute host path is what kandev sends.
- Bind-mount /home/kandev/.kandev so per-session agentctl logs
survive --rm teardown and the test runner can inspect them on
failure.
- Replace the broken CPU-spin sleep helper with a real sleep
subprocess and a /dev/tcp TCP-open probe so timing-dependent
helpers stop racing the container start.
- Retry the host fingerprint scan with stderr capture; the first
scan can race the sshd accept loop on Alpine.
Backend SSH executor (apps/backend/internal/agent/lifecycle):
- executor_ssh_operations.go startRemoteAgentctl: agentctl does not
accept --port-file or --pid-file flags (it reads AGENTCTL_PORT
and writes its own pid). Rewrite the launch wrapper to inject
AGENTCTL_PORT via env, capture $! as the pid, and tail the
on-remote agentctl.log for 'HTTP server bound successfully'
instead of waiting on imaginary side-files.
- executor_ssh_operations.go createRemoteAgentInstance: new helper
that POSTs /api/v1/instances on the in-container control server
via SSH-exec'd curl, returning the per-instance port the executor
should forward to. Mirrors what executor_sprites_operations.go
does inside its sprite. Without this step the agent stream
returns 'configure request failed with status 404' because the
SSH executor was talking to the control server, not the
per-session sub-server.
- executor_ssh.go startAndForwardAgentctl: chains the new
create-instance step in between agentctl launch and port-forward,
and forwards the *instance* port (not the control port).
- executor_ssh.go: drop the per-host SSH connection pool (it was
introducing a subtle aliasing issue between
CreateInstance/StopInstance/GetRemoteStatus that made the SSH
client unreliable across same-host sessions). Each session now
holds its own ssh.Client directly on sshSessionState and
ResumeRemoteInstance dials a fresh client too.
- executor_ssh_operations.go SSHPortForwarder: serialize the
client.Dial calls with a per-forwarder mutex. golang.org/x/crypto/
ssh.Client.Dial is documented as concurrency-safe but in practice
parallel opens from the kandev stream-manager occasionally
produce io.EOF as if the channel-open response was never
received.
Test fix (apps/web/e2e/tests/ssh/error-surfacing.spec.ts):
- Q1 ('TCP refused') was using identity_source='agent' with no
SSH_AUTH_SOCK in the backend env, so it failed at
buildAuthMethods before reaching the TCP-refused path. Switch to
identity_source='file' with the seed key so the test exercises
the failure it claims to.
Known remaining issue (to investigate next): the launch path now
reaches 'agent process started' + 'ACP initialize response
received' when run in isolation, but the WebSocket updates stream
sometimes disconnects with 1006 abnormal closure shortly after. A
standalone Go reproduction of concurrent direct-tcpip channels
against the same image works without issue, so the trigger is
something specific to the kandev backend's stream-manager + my
forwarder interaction. Tracked for the next iteration.
* fix(ssh): half-close forwarded SSH channels to avoid premature WS abnormal-close
The SSH executor's port-forwarder previously called the full Close() as soon
as either direction of the bidirectional copy finished. Under HTTP-Upgrade
WebSocket traffic this meant: when the WS server flushed its final frame and
shut down the write side, our local->remote io.Copy returned EOF, we closed
the entire SSH channel, the remote->local copy aborted mid-frame, and the
backend's WS reader observed close 1006 (unexpected EOF) instead of a clean
close frame.
Switch to a half-close pattern: each direction CloseWrite()s its destination
when its source EOFs, letting the other half drain the remaining frames. The
final Close happens after both copies return.
The "ssh forwarder dial remote failed: EOF" warning in e2e logs was a
follow-on symptom — the SSH client was being torn down by the abnormal close,
so any later channel-open returned EOF immediately.
* fix(ssh): resolve session executor_id via TaskSession join
GET /api/v1/ssh/executors/:id/sessions was filtering ExecutorRunning rows by
their ExecutorID column, but the lifecycle manager's buildRunningFromExecution
only sets that column from a prior row — for fresh sessions it's left empty.
The endpoint therefore returned [] for every just-launched SSH session.
Look up the TaskSession by session_id and compare its ExecutorID instead.
Keep the row's own ExecutorID as a fast-path for resume/recovery flows
where it's already populated.
* feat(web): register ssh entry in the new-executor type map
Without this entry, /settings/executors/new/ssh fell back to the
"unknown executor type" placeholder. Extracts the type map into its
own module so the page file stays under the 600-line lint cap.
The SSH-specific form is still TODO and will replace the standard
profile-create form for this type in a follow-up.
* test(e2e): poll listSSHSessions for SSH session row in group G
The simple-message scenario completes inside a couple hundred
milliseconds, so the previous "wait for executor_type=ssh, then
query once" pattern raced the row's removal during teardown. Poll
listSSHSessions directly with a 60s window so the row is observed
while the executor is up.
Adds dumpRemoteLogs helper for copying per-session agentctl.log
files out of the sshd container on assertion failure, easing
debugging when the row never appears.
* test(e2e): align H5 assertion with actual on-disk session-dir layout
The wrapper writes agentctl.pid and agentctl.log to <sessionDir>; the
remote agentctl port travels via AGENTCTL_PORT env + ExecutorRunning
metadata, not a separate .port file. Assert the files that actually
exist and the metadata field that carries the port.
* feat(web): mount SSH connection + sessions cards on dedicated routes
Wires the existing SSHConnectionCard and SSHSessionsCard into the
settings UI so the SSH executor flow has a home:
- /settings/executors/new/ssh renders SSHCreatePage (just the connection
card; Save POSTs a new executor and routes to the edit page).
- /settings/executors/ssh/[executorId] loads the executor and renders
the connection card pre-filled + the sessions card.
- Adds the SSH entry to the executors landing-page tile grid.
- Adds createExecutor / fetchExecutor / updateExecutor helpers to
settings-api so the new pages can talk to /api/v1/executors directly.
Profile management for SSH executors still flows through the existing
profile pages; this commit only adds the executor-level chrome the
SSHSettingsPage e2e tests expect.
* fix(web): keep SSH trust checkbox visible after editing a connection field
Previously, editing any form field nulled out `result` so the entire
TestResultDisplay (including the trust checkbox) unmounted. The trust-gate
e2e specs assert that after editing host/port/user/identity_source, the
trust checkbox is still present but unchecked — which matches the
intended UX: the user can re-test against the new target and re-tick
trust without losing the prior result for reference.
Now an edit only clears `trust` when the field could route to a
different machine (host_alias, host, port, user, identity_source,
identity_file, proxy_jump); the result panel stays mounted. Editing the
display-only `name` leaves trust alone.
* test(e2e): point SSHSettingsPage at the new SSH executor routes
Updates the page object to navigate to the SSH-specific routes that now
host the connection + sessions cards:
- gotoNew(): /settings/executors/new?type=ssh → /settings/executors/new/ssh
- gotoExisting(): /settings/executors/[executorId] → /settings/executors/ssh/[executorId]
The old URLs collided with the standard profile-create / profile-edit
flow and were never going to render the SSH cards. The new paths give
the SSH UI its own surface, leaving the generic profile pages untouched.
* feat(api): add top-level DELETE /api/v1/executor-profiles/:profileId
Profile IDs are globally unique, so callers that only have the
profile id (e2e api client, CLI tooling) shouldn't have to plumb
the executor id through. Aliases the existing nested handler.
* test(e2e): probe SSH fingerprint via kandev, not ssh-keyscan, in K specs
regenerateHostKey was returning the ssh-keyscan fingerprint, but
ssh-keyscan and the Go ssh client can negotiate different host-key
types — meaning the fingerprint kandev observes during a real probe
does not always match what keyscan saw. The keyscan output was used
verbatim as the expected value, which K1 / K3 then asserted against
kandev's pinned-fingerprint result, so the specs flapped depending
on which algorithm sshd advertised.
Drop the return value from regenerateHostKey and have callers hit
/api/v1/ssh/test to learn the canonical fingerprint, matching what
the SSH fixture already does at seeding time.
K2 was polling env.error_message which doesn't exist — the launch
fails at SSH dial time before the executor is recorded on the task
environment. Point the polling at the task-session error_message
instead, where updateTaskSessionState actually persists the dial
failure.
* test(e2e): wipe agentctl before F5 to assert from a fresh-install state
F5 asserts that the agentctl_action probe reports "uploaded" before
the first launch and "cached" after. Tests share a worker-scoped
sshd container, so an earlier test in the same worker may have
already primed the cache, making the "before" assertion flap.
Wipe ~/.kandev/bin/agentctl + .sha256 from the container at the top
of F5 so the assertion sees the documented fresh-install path.
Export execInContainer so the spec can reach into the container
without copying the docker-exec boilerplate.
Add error_message to the listTaskSessions return type so the
hostkey-rotation spec can read the field (which the backend already
returns).
* fix(ssh): re-read ~/.ssh/config on every resolve, no process-wide cache
kevinburke/ssh_config's package-level Get/GetStrict use a sync.Once
to load $HOME/.ssh/config once per process and cache the result. That
breaks two things:
* users who edit their config mid-session don't see updates until
kandev restarts;
* e2e tests that write a fresh ~/.ssh/config per test all see the
state captured by whichever test triggered the first lookup.
Open and Decode $HOME/.ssh/config on demand from a local
inheritFromSSHConfig helper. The library still parses the file the
same way — we're just dropping the global cache.
Unblocks the four ssh/config-inheritance.spec.ts cases that wrote a
per-test ~/.ssh/config.
* feat(ssh): parse literal user@host:port ProxyJump bastion strings
dialViaJump fed target.ProxyJump straight into ResolveSSHTarget as a
host alias. That works for `Host prod-jump` entries in ~/.ssh/config
but turns the literal `user@host:port` form into a single hostname
that the kernel resolver chokes on (`lookup kandev@127.0.0.1:23000: no
such host`).
Detect the literal shape inline (`[user@]host[:port]`) and build the
bastion SSHTarget from its parts. Falls back to the alias path when
no @ / : separators are present.
Fixes the proxy-jump.spec.ts bastion flow. While here:
* scanFingerprintInsideNetwork: alpine package is `openssh-client`,
not the (nonexistent) `openssh-keyscan`. Surface install errors
so future failures aren't silent.
* scanFingerprintViaHost: retry on empty output — sshd accepts TCP
before host-key negotiation completes on a cold container.
* startBastionAndTarget: share one keypair across bastion + target
so a single -i auths both hops (matches how OpenSSH ProxyJump
handles per-host identities outside ~/.ssh/config).
* proxy-jump N1: assert any SHA256 fingerprint came back, since the
test no longer pre-discovers a value to compare against.
* test(e2e): time-bound the drop-traffic SSH probe with an AbortController race
iptables -A INPUT -p tcp --dport 22 -j DROP silently discards SYN
packets — Go's net.Dialer sits in the connect syscall through the
kernel's TCP_SYN_RETRIES (default 6 ~ 127s) before the syscall is
abandoned, even though the dialer's logical Timeout is 30s. The
spec polled the testSSHConnection API for the failure path; the
request blocked long enough that Node's keep-alive evicted the
socket and the test failed with "other side closed" instead of
seeing kandev's success=false response.
Race the probe against a 5s timeout. If the request errored or
timed out the test passes (both are valid evidence that the drop
hit the dial path); if it came back with a response, assert
success=false the same way the original spec did. The restore-probe
poll still gates on a real success after restoreTraffic.
* test(e2e): rerun ssh.test in hostkey-rotation afterEach to repin
Hostkey-rotation specs mutate the worker-scoped sshd container and
the seeded SSH executor's pinned fingerprint. Without a cleanup hook,
the next spec in the same worker that resolves seedData.sshTarget.
hostFingerprint observes a stale value and the UI's trusted-badge
check (persistence R1) flaps.
Add an afterEach that re-probes the live container via kandev, syncs
the fingerprint into both seedData and the seeded executor's pinned
config. The cleanup is a no-op when the probe fails (e.g. K2 left an
intentionally bad fingerprint that no longer matches anything — the
next test will re-establish state via its own setup).
* test(e2e): restore the seeded executor's pinned fingerprint after R5
The fingerprint-change-warning spec pinned a deliberately bogus
fingerprint on the worker's seeded SSH executor to exercise the
mismatch UI, then exited without resetting it. Later specs in the
same worker that launched real tasks against the same executor
(workdir-per-profile R1/R2 in alphabetical order) failed at SSH dial
time with host-key-changed, since the executor was still pinned to
the bad value.
Wrap the assertion in a try/finally that restores the original
fingerprint from seedData when the test ends, so the worker leaves
the executor in the same state it inherited.
* fix(ssh): quote every remote path + clean identity file path
CodeQL flagged two paths from HTTP-supplied SSH-config fields into
shell + filesystem operations:
* runSSHCommand built "cat $remoteShaFile" / "test -x $remoteBin"
/ "mkdir -p $(dirname $remoteBin)" by direct concatenation; the
arguments come from $HOME on the remote (not the kandev user)
but the analyzer can't see that, and a remote $HOME containing
shell metacharacters would still misparse. Route them through
shellQuote for parity with every other call site in the file.
* dialAuth opened target.IdentityFile straight from the configured
string. Run it through filepath.Clean so any `..` segments slipped
in by string substitution are normalised before os.ReadFile, which
is the canonical mitigation CodeQL recognises here. Behaviour is
unchanged for normal paths.
* refactor(ssh): drop the unused conn pool + remote curl dependency
CodeRabbit and Greptile both flagged the SSHConnPool: the executor never
took the pool path (CreateInstance always dialDirect'd a fresh client and
parked it on sshSessionState), so the pool's keepalive goroutines and
refcount logic were dead code. Get() also overwrote map entries on every
call, which would have leaked keepalive goroutines if the pool *had* been
used. Delete the entire SSHConnPool + sshConnKey + pooledConn + keepalive
plumbing. Export DialSSH so the SSH test endpoint can dial directly and
close the client when its single-shot probe is done.
While here:
* pickRemoteAgentctlPort: switch from time.Now().UnixNano()%20000 to
math/rand/v2.IntN. Two concurrent CreateInstance calls on a fast
machine were getting the same port (UnixNano cycles in ~20µs through
the 20k range); randomized picks avoid the collision.
* createRemoteAgentInstance: POST to the remote control port over a
direct-tcpip dial via net/http instead of shelling out to `curl`. The
sshd container no longer needs curl, the kandev test endpoint no
longer needs to probe for it, and the call inherits Go's standard
timeout + error handling. Drop `curl` from the e2e Alpine image.
* maybeUploadCredentials: was probing `printf %s "$HOME"` and
discarding the output, then allocating an unused sshFileUploader.
SSH v1 doesn't ship credential upload — just log a warning when a
user sets remote_credentials so they know it won't take effect, and
delete the dead uploader file.
* fix(ssh): verify bastion host key + tighten the trust-gate UX
Bastion host-key verification (security)
resolveBastion built the bastion's ClientConfig with an empty
PinnedFingerprint, which buildClientConfig translated into a
callback that accepts any key. An attacker on the path to the
bastion (DNS poisoning, route hijack) could pivot through it
before the final-target check ever ran. Greptile flagged it as P1.
Swap in a callback that consults ~/.ssh/known_hosts via
knownhosts.New: matched key → accept; mismatched key → reject loud
(the alarm); host absent → accept with the same TOFU semantics
OpenSSH uses for StrictHostKeyChecking=accept-new, so e2e and
first-connect setups still work without manual `ssh-keyscan`
priming.
Stale-test trust gate (frontend)
Editing a connection field after a successful test previously kept
the prior result + cleared `trust`, but the trust checkbox could
be re-checked and saved with the old fingerprint bound to the new
target. CodeRabbit + cubic both flagged it as P1.
Add a `resultStale` flag that flips when a CONNECTION_FIELD edit
follows a successful test, surface a warning under the result, and
disable the trust checkbox + save button until the user re-runs
Test Connection. handleTest clears the flag.
While here
* Surface onSave failures in the card instead of swallowing them.
* ssh-create-page: read the executors slice from the live store
(useAppStoreApi + getState()) at write time so a concurrent
update doesn't get overwritten by a stale closure.
* Split FingerprintTrustBlock into its own file to keep
ssh-connection-card under the 600-line cap.
* fix(ssh): defend launch + handshake against several real bugs
manager_launch: trusted exec-config keys override req.Metadata.
buildLaunchMetadata applied executor config only when the matching
key was missing in req.Metadata. A request that carried metadata.
ssh_host_fingerprint (or ssh_host, ssh_identity_*) could pivot the
launch to a different target and bypass the pinned fingerprint.
Pin a list of connection-routing keys (ssh_host, ssh_port, ssh_user,
ssh_host_fingerprint, ssh_identity_source, ssh_identity_file,
ssh_proxy_jump) and have the executor config always win for those.
Cubic / CodeRabbit P1.
executor_ssh.CreateInstance: reuse the resumed session state.
ResumeRemoteInstance attaches the recovered ssh.Client + forwarder
to r.sessions[InstanceID] and refreshes metadata, then the lifecycle
manager unconditionally called CreateInstance, which dialed a fresh
client and started a second remote agentctl on top of the live one
— leaking the resumed state. CreateInstance now early-returns from
the existing r.sessions entry via buildResumedInstance. Cubic P1.
executor_ssh.Close: actually release tracked sessions.
After dropping the SSHConnPool, Close was a no-op. A hard kandev
exit (no per-session StopInstance) left forwarders and SSH clients
open. Iterate r.sessions and Close the forwarder + client for each.
Cubic P2.
buildAuthMethods: close the ssh-agent unix socket after handshake.
Each agent-authenticated dial opened net.Dial("unix", $SSH_AUTH_SOCK)
and never closed it; only PublicKeysCallback held the agent.Client
reference and that callback is consulted exactly once at handshake.
Return a cleanup func from buildAuthMethods + buildClientConfig and
defer it from dialSSH / dialViaJump so the socket releases as soon
as key exchange completes. Greptile P1 security.
waitAgentctlHealthy: probe HTTP /health, not just the TCP listener.
The local port is owned by the SSH forwarder, which Accepts before
it Dials the remote direct-tcpip channel — a TCP connect can succeed
before the channel actually establishes. Issue a real GET /health
and require a 2xx so a broken tunnel surfaces here rather than at
the first agent operation. Cubic P1.
ssh-connection-card: also gate the trust + save on in-flight tests.
Editing a connection field during a pending test silently kept
resultStale=false because prev.result was still null; when the test
returned, the new fingerprint was bound to the old form values but
the gate let the user trust + save anyway. Treat in-flight tests as
prior tests for staleness purposes, and preserve resultStale across
the test's success setState. Cubic P2.
* fix(ssh): drop stale polls + preserve port inheritance + sshd extra-keys
ssh-sessions-card: race-protect polls.
Polling setInterval + manual refresh shared one state setter, so a
slow request that resolved after a newer call (or after a parent
switched executorId) overwrote fresh data with stale rows. Bump a
monotonic sequence on each refresh and drop the response when the
sequence has moved on, plus reset on mount and invalidate on
unmount so an in-flight reply from a teardown can't land. Cubic P2.
ssh-config parseSSHExecutorConfig: don't default ssh_port to 22.
Reading an executor row that had no ssh_port set rewrote port=22
on the form, which then round-tripped back to the backend. That
silently broke ~/.ssh/config Port inheritance for aliases — a save
could redirect connections to 22 even though the underlying alias
was supposed to resolve to another port. Leave the field undefined
so the backend resolver can apply the alias' value. Cubic P2.
ssh-image entrypoint: guard authorized_keys.extra mount.
When only /authorized_keys.extra (and not /authorized_keys) was
bind-mounted, the >> redirect ran under set -e against a missing
destination and aborted container startup. mkdir + touch the
destination before the append and reapply the ownership/perms.
CodeRabbit P2.
* test(e2e): make G2 + F6 actually exercise the contract
G2 (sessions-endpoint filter) iterated `for (const s of sessions)` over
the listSSHSessions result without first asserting `length > 0` — an
empty response let the test pass without checking anything. Seed an SSH
launch before the loop and require at least one row, so a handler
regression that returned [] would now fail loud. CodeRabbit P1.
F6 (ws ssh.test parity) called apiClient.testSSHConnection twice (HTTP
both times) and compared the two HTTP responses, which would have
passed even if the WS dispatcher entry was broken. Use apiClient.
wsRequest("ssh.test", payload) for the second call so the gateway's
WS handler is actually invoked, and assert the step names match across
transports. CodeRabbit P1.
* refactor(e2e): extract SSH HTTP shapes into api-client-ssh.ts
api-client.ts was already over the 600-line lint cap on main (1528
lines); the SSH additions made it worse. Split the SSH HTTP shape
types into a dedicated module and re-export from api-client.ts so
existing callers keep working. The methods themselves stay on
ApiClient as thin pass-throughs since splitting them out across the
class boundary would change every test call site.
Also document the per-executor bound on the SSH listSessions per-row
GetTaskSession lookup (typical N < 10), and what a batch path would
need (a repo method to fetch task_sessions by id list). Cubic /
Greptile P2.
* docs(spec): correct SSH recovery keying after pool removal
The spec said ResumeRemoteInstance re-opens 'per (host, user) — one
connection serves all surviving sessions on that host', which was
the pool design. The current executor gives each session its own
SSH client, so the resume keys on the full (host, port, user,
identity_*, proxy_jump, host_fingerprint) tuple. That way two
profiles on the same host with different keys can resume side by
side without merging. Cubic P2.
* fix(ssh): IPv6 ProxyJump + port range + dedup + docker timeouts
parseLiteralProxyJump: handle bracketed IPv6 + reject invalid ports.
The literal parser used LastIndex(":") to split host from port, which
for [2001:db8::1]:22 left host as "[2001:db8::1]" and produced an
invalid double-bracket address through net.JoinHostPort. Recognise
the bracketed form explicitly via a parseBracketedHostPort helper
and strip the brackets. Validate port is in [1, 65535] instead of
accepting any integer (Atoi happily parses negative or out-of-range
values). Split the host/port branch into its own function so the
nesting stays under the golangci nestif limit.
ssh-create-page: dedupe on append.
setExecutors([...current, next]) would double-list the same executor
if a WS event (or another refresh) inserted it while createExecutor
was in flight. Skip the append when current already has the id.
ssh-image fixture: time-bound docker probe + build.
hasSSHContainerSupport ran `docker info` with no timeout — an
unreachable / hung daemon would stall the Playwright worker
indefinitely. Cap at 10s for the probe. buildE2ESSHImage runs
`docker build` with no timeout; cap at 15 min so a stuck registry
pull or BuildKit hang fails fast.
* fix(ssh): reject invalid ssh_port + retry in-net keyscan + restore D5 + doc count
executor_ssh.targetFromMetadata: reject malformed / out-of-range
ssh_port instead of silently falling back to 0 → 22 in the resolver.
"abc", "0", "70000" all now return a "must be 1-65535" error so a
broken config can't masquerade as a successful launch routed through
port 22. Cubic P2.
scanFingerprintInsideNetwork (bastion fixture): retry on empty output.
Mirrors the host-side scanFingerprintViaHost retry loop — sshd accepts
TCP before host-key negotiation completes on a cold container, so the
keyscan can come back empty on the first try. Bound at 15s; surfaces
"status=..., stderr=..." on final failure for diagnosis. Cubic P2.
executor-crud D5 (changing config…): restore the seeded executor in a
try/finally. The test pinned the executor to host=10.255.255.1 to
verify the snapshot stayed sticky on the live session, but never put
the real host back, so any later spec in the same worker that reused
seedData.sshExecutorId would dial the unroutable address and hang.
Cubic P2.
e2e-plan.md: replace the "roughly 75" estimate with the actual 80 /
17-files current count plus the ~2.4 min wall-clock. Cubic P3.
* fix(ssh): use bare mock-agent name so the remote PATH resolves it
For SSH the agent runs on a remote host whose filesystem doesn't share
paths with the kandev backend's, so the host-absolute mock-agent path
configureMockAgent computes can't apply. Extend the bare-name branch in
MockAgent.BuildCommand to include RuntimeSSH (the SSH e2e image already
bakes mock-agent into /usr/local/bin) and drop the now-redundant SSH e2e
bind-mount that worked around this by mirroring the host path into the
container.
* fix(ssh): unique sftp temp filename so concurrent uploads don't collide
When two sessions on the same task launch back-to-back they both call
ensureAgentctlOnHost which streams to <remotePath>.tmp before renaming.
With a shared .tmp name the second uploader's rename errors with
"file does not exist" once the first uploader's rename consumed the temp
file. Give each uploader its own pid+random-suffix temp; uploaders write
identical sha256-keyed content, so the rename race is then safe.
* fix(ssh): don't leak session-scoped metadata across sibling sessions
reuseExistingEnvironment copies persistent metadata from the previous
ExecutorRunning row into the new launch request so executors can pick up
worktree/container/sandbox IDs. For SSH that swept in session-scoped
keys too — agentctl PID/port, session dir, local forward port — which
made the second session's ResumeRemoteInstance reattach to the first
session's agentctl process. Mark those keys as session-scoped and skip
them in applyExecutorRunningMetadata; the same-session resume path
(applyRunningRecordToResumeRequest) still copies them because it works
off the session's own ExecutorRunning record.
* test(ssh): cover ProxyJump parsers + sibling-session metadata filter
Addresses the two coverage gaps Claude flagged on PR review round 3:
- parseLiteralProxyJump / parseProxyJumpHostPort / parseBracketedHostPort
/ parsePortString are pure parsers that gate every ProxyJump dial,
including IPv6 (`[2001:db8::1]:22`). Table-driven tests pin the happy
paths and the bracket / port / range edge cases — silent regressions
in bracket stripping would otherwise break every IPv6 ProxyJump
session without a single test failing.
- applyExecutorRunningMetadata is the function commit
|
||
|
Carlos Florêncio
|
68aefc0e08 | feat(web): system settings pages (status, database, backups, logs, updates, licenses, about) (#942) | ||
|
Zaybrah
|
ec21c71775 | feat(agentctl): add Azure Repos PR creation (#1066) |