d855d615f854429b0cc3fa4cbbe995bdb8f28a60
11 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Carlos Florêncio
|
3a1ca57c0a | fix: route PR-only commits to GitHub details (#2268) | ||
|
Carlos Florêncio
|
b39a668e99 | feat(github): add task git credential policy (#1985) | ||
|
José Almeida
|
d5da5d0022 |
fix(web): select pull request on review surface (#1857)
* fix(web): select pull request on review surface * fix(web): address review feedback * test(web): stabilize tablet review target assertion * fix(web): address late review feedback * test(web): cover PR review boundary branches |
||
|
José Almeida
|
e067e7a1f6 |
feat(web): show file statuses in review (#1683)
* feat(web): show file statuses in review * fix(web): address review status feedback * fix(web): deduplicate review sources * fix(web): align review progress identities * fix(web): guard review status labels * fix(web): enforce file status exhaustiveness * perf(web): index review progress sources * fix(web): preserve files during status hydration * fix(web): align multi-repo review progress |
||
|
José Almeida
|
b277dcdcdf |
fix: simplify walkthrough prompt requests (#1660)
* fix: simplify walkthrough prompt requests * fix: address walkthrough review feedback |
||
|
José Almeida
|
3e237e177a |
feat: add agent-authored code walkthroughs (#1647)
* feat: add code-walkthrough backend (MCP tool, service, persistence, WS events) * feat: render agent code walkthroughs as line-anchored cards + editor window * test: add code-walkthrough re-emit mock-agent scenario * fix: rework code-walkthrough UX (floating card, line reveal, minimize, clickable file) * fix: improve code walkthrough editor UX * fix: backfill walkthrough launcher after websocket connect * fix: improve walkthrough requests and editor anchors * fix: clear walkthrough highlights on close * feat: add walkthrough discard and prompt customization * fix: send walkthrough requests through saved prompt * fix: support postgres built-in prompt seeding * fix: address walkthrough review feedback * fix: address walkthrough review follow-ups * fix: harden walkthrough lifecycle races * fix: reuse walkthrough notification cleanup * fix: guard stale walkthrough file opens * fix: abort stale file opens before paint * fix: stabilize github settings e2e readiness * fix: use stable github settings e2e selector |
||
|
José Almeida
|
9c73a2d8c3 |
fix: keep diff previews tied to the correct file (#1618)
* fix: keep diff previews tied to the correct file * fix: handle legacy file editor panel restores * test: align file editor image mock * fix: ignore sibling repo editor sync changes * test: use repo scoped file keys in save tests * fix: guard file editor preview restoration * fix: remove redundant preview path fallback * test: cover repo scoped image previews * test: use repo scoped keys in sync tests * fix: guard stale file editor open fetches * fix: distinguish repeated file editor requests * fix: open single-repo PR diffs without repo filter * fix: keep pending file opens alive |
||
|
João Salavisa
|
d83c1dd982 |
feat(diff): use stored base_branch for diff stats + per-task compare picker (#1273)
* feat(diff): use stored base_branch for diff stats + per-task compare picker
Task-card additions/deletions and the Changes panel were computed against a
hardcoded origin/main → master priority list inside agentctl. For tasks
branched off a release/staging branch this inflated the counts by every
commit since the actual fork point, and the Changes panel grouped unrelated
files alongside the task's real work.
This wires the per-repo task_repositories.base_branch end-to-end into the
agentctl WorkspaceTracker so each repo's diff stats resolve against the
recorded base instead of the integration-branch fallback. A picker on the
Changes panel branch hover card lets the user change the base mid-task; the
new value is persisted via PATCH /tasks/:id/repositories/:repo_id, pushed
live to the running agentctl instance, and the session's cached
base_commit_sha is cleared so the commits panel + cumulative diff refetch
against the new base without a session restart.
Falls back to the existing origin/main → master list for legacy tasks
(empty base_branch) and to the branch tip when no merge-base exists with
HEAD (unrelated histories — local rebase backups, etc.) so the stats and
commits panels stay anchored on the same SHA in every failure mode.
* fix(agentctl): sanitize user-controlled base_branch refs before git exec
CodeQL flagged the picker's user-controlled base_branch payload reaching
exec.Command("git", ..., baseBranch) inside workspace_git_cmd.go as a
command-built-from-user-controlled-source vulnerability. A malicious caller
could smuggle a leading-"-" argument like "-upload-pack=..." that git
itself reinterprets as a flag, even though exec.Command keeps args
separate from the shell.
Adds an IsSafeGitRef sanitizer at every boundary the value crosses on its
way to git: the service write path, the agentctl HTTP handlers
(/workspace/base-branches, /git/log, /git/cumulative-diff), and
WorkspaceTracker.SetBaseBranch itself. Unsafe values are rejected at the
service tier and silently downgraded to the no-override fallback inside
agentctl so a misbehaving caller can't half-break the tracker.
* fix(diff): address PR review blockers and CI lint
- Split UpdateRepositoryBaseBranch into validate/load/apply helpers to drop
cyclomatic complexity below the cyclop limit that gated Run Backend Tests.
- Replace direct m.cfg.BaseBranches reads/writes with getBaseBranches /
setBaseBranches under a dedicated RWMutex. UpdateBaseBranches used to
write under repoTrackersMu while rescan + lazy-subpath callers read under
workspaceTrackersMu, leaving an unsynchronized data race on the map.
- Drop the false 'PR target is auto-wired' claim from the MCP tool
description — GitOperator.CreatePR takes baseBranch as an explicit arg
and never reads the tracker's value.
- Picker no-op guard now compares against currentBase instead of storedBase
so selecting the displayed fallback (legacy task before any picker
change) is treated as no-op instead of triggering a spurious save.
- HTTP handler now maps non-validation service errors to 500 instead of
400, classifying via the existing isValidationError helper plus the new
unsafe-git-ref message.
* fix(diff): regex-based ref sanitiser at every git subprocess sink
CodeQL kept flagging workspace_git_cmd.go:43 even after SetBaseBranch
rejected unsafe values — the taint tracker doesn't follow sanitised stores
through struct fields, so it still saw user input flowing into
exec.Command("git", ..., baseBranch).
Switches IsSafeGitRef to a single regex check (a pattern CodeQL recognises
as a sanitiser barrier) and adds SanitizeGitRef calls inline at every
agentctl call site that splices a user-controlled ref into a git arg:
- workspace_git_status.resolveStoredRef (before the rev-parse probes)
- workspace_git_status.computeBaseCommit (before merge-base/rev-parse)
- workspace_git_status.getAheadBehindCounts (before rev-list)
- agentctl/server/api/git.go runGitLogForRepo + handleGitCumulativeDiff
(before computeMergeBase/GetRevParse on the HTTP TargetBranch)
Behaviour is unchanged for valid refs — only narrows the allowlist (now
[A-Za-z0-9_./-]) so the sanitiser barrier is recognisable to static
analysis. SetBaseBranch's field-store sanitiser stays as defence in depth.
* fix(diff): transform sanitiser at every map read for CodeQL taint flow
CodeQL still flagged workspace_git_cmd.go:43 because:
- handleSetBaseBranches used an IsSafeGitRef bool guard before a verbatim
map[k] = v copy. CodeQL recognises regex-transforming sanitisers but not
bool-guarded passthroughs.
- lookupBaseBranch returned m.cfg.BaseBranches[k] verbatim. Even though
the map was sanitised on write, taint flow across map stores + field
stores loses the sanitised state in the analyser.
Switches both sites to the transforming SanitizeGitRef helper so every
read produces a freshly-sanitised value before it flows to a downstream
git argument. Behaviour is unchanged for valid refs.
* fix(diff): regex transformer sanitiser + guard live push from empty map
CodeQL still flagged the previous SanitizeGitRef because it returned the
input string unchanged on success — an identity passthrough that the taint
tracker doesn't recognise as a sanitiser barrier.
Switches to safeGitRefPattern.FindString so the output comes from the
regex match (not the parameter). IsSafeGitRef now wraps SanitizeGitRef so
the predicate and the transformer share one allowlist.
Also addresses two Claude review blockers:
- collectTaskBaseBranches returns a nil/empty map when the task has no
recorded base_branches; pushing that to agentctl wipes every tracker's
override. Now skipped — the DB row is the source of truth for the next
session launch.
- applyBaseBranchSideEffects now runs on context.WithoutCancel so an HTTP
/ WS request cancel after the task_repositories UPDATE commits can't
leave session.base_commit_sha stale + the live agentctl tracker map
un-pushed, drifting from the persisted row.
* fix(diff): share securityutil.IsValidBranchName + harden error classification
CodeQL still flagged the picker's regex sanitiser as taint passthrough.
Refactors SanitizeGitRef / isSafeBaseBranchRef to delegate to the existing
securityutil.IsValidBranchName helper that agentctl's Rebase / Merge /
RenameBranch already use without being flagged. The shared allowlist also
fixes a Claude review blocker: 'feature/@2024' used to pass the service
validator but get dropped by the agentctl-side check at push time,
silently wiping every tracker's override.
Plus the two error-classification blockers Claude raised on the previous
HEAD:
- WS handler (task_ws_handlers.go wsUpdateTaskRepository) now distinguishes
validation errors (surfaced verbatim) from internal faults (opaque
500-style message). HTTP path already did this via the existing
isValidationError helper.
- MCP handler (handlers.handleUpdateRepositoryBaseBranch) gets the same
treatment via a new isValidationError local to the mcp/handlers
package so raw service error strings stop leaking across the MCP
boundary.
* fix(diff): inline securityutil.IsValidBranchName at every git subprocess sink
CodeQL's interprocedural taint tracking wasn't recognising SanitizeGitRef
as a sanitiser even after it delegated to securityutil.IsValidBranchName
— the wrapper helper + map/field hops between source and sink kept the
flow tainted in the analyser.
Inlines the IsValidBranchName allowlist check (already used inline by
Rebase/Merge in git.go without being flagged) at every call site that
splices a user-controlled ref into a git subprocess argument:
- workspace_git_status.resolveStoredRef (before rev-parse --verify)
- workspace_git_status.computeBaseCommit (before merge-base / rev-parse)
- workspace_git_status.getAheadBehindCounts (before rev-list)
- agentctl/server/api/git.go runGitLogForRepo + handleGitCumulativeDiff
- agentctl/server/api/workspace_base_branches.go handleSetBaseBranches
Behaviour unchanged: same allowlist as Rebase / Merge / RenameBranch
already use. SanitizeGitRef / IsSafeGitRef survive as defence-in-depth
helpers for callers that want a one-line check; they delegate to the same
securityutil helper so the rules stay aligned.
* fix(diff): inline regexp.MatchString at every git subprocess sink
CodeQL still flagged workspace_git_cmd.go:43 even with the inline
securityutil.IsValidBranchName helper — the taint tracker apparently does
not follow into helper functions for the go/command-injection rule on
this path.
Hoists the regex to package-scope vars (safeBranchRefPattern in both
process and api packages) and calls regexp.MatchString directly at each
sink site. Behaviour is identical to securityutil.IsValidBranchName; only
the call form changes.
* fix(diff): SHA-shape allowlist on BaseCommit before downstream git diff
CodeQL still flagged workspace_git_cmd.go:43 because update.BaseCommit
is the OUTPUT of an earlier merge-base / rev-parse call whose argument
list contained a user-controlled branch ref — the analyser treats that
command's output as still tainted even when the input ref was sanitised.
Adds an inline 40-hex SHA-shape allowlist check in enrichWithBranchDiff
right before `git diff --numstat <update.BaseCommit>` so the sanitiser
barrier sits in the same function as the next subprocess call. Behaviour
is unchanged: merge-base / rev-parse already produce SHA-shaped output.
* fix(diff): async tracker refresh + tighten init spread order
Two CodeRabbit/Claude review fixes:
- UpdateBaseBranches stamps each tracker's new baseBranch synchronously
then runs RefreshGitStatus on a background goroutine instead of inside
the picker-save HTTP handler. Per-tracker git subprocesses can each
take ~3-5 calls; serialising them on the request goroutine made the
PATCH response wait for every refresh to finish. The picker UI re-fetches
via the existing WS stream once Refresh emits, so the handler doesn't
need to block.
- updateTaskRepositoryBaseBranch now spreads options.init BEFORE the
required method/body fields so an upstream caller's init overrides
can't replace the PATCH method or the base_branch payload. Matches
the convention deleteTask / moveTask already follow.
* fix(diff): narrow mcp isValidationError + opaque 500 + post-sanitize log count
Three final Claude review threads on the latest commit:
- mcp/handlers isValidationError drops the broad 'invalid' substring so
DB / IO errors whose messages happen to contain 'invalid' don't
misclassify as ErrorCodeValidation.
- task_http_handlers httpUpdateTaskRepository now logs the raw service
error server-side and returns an opaque 'failed to update task
repository' on the 500 path. Stops potential DB / IO detail leakage
through the HTTP boundary.
- workspace_base_branches debug log reports accepted + rejected counts
(post-sanitisation) instead of the pre-filter total so the metric
actually reflects what hit UpdateBaseBranches.
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
|
||
|
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
|
5f24349aae | feat: show relative commit time on hover in changes panel (#1199) | ||
|
Carlos Florêncio
|
8a7b7a0b60 | feat(agentctl): azure Repos PR follow-ups after #1066 (#1071) |