a8af0787b18a2b6e4170efd0e4bb82425401b2ee
87 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Clément Fiere
|
3e794ba903 | fix(github): stop SELECT * scans from breaking on TaskPR schema drift (#1988) | ||
|
Luan Martins
|
04a5c4e4ee | feat(github): add task PR lifecycle notifications (#2038) | ||
|
Carlos Florêncio
|
1f19dbe6d5 | fix: keep watcher save state visible (#2037) | ||
|
Carlos Florêncio
|
b39a668e99 | feat(github): add task git credential policy (#1985) | ||
|
João Salavisa
|
37b56ba11c |
feat(automations): reliable cron scheduling and webhook-driven push/CI triggers (#1972)
* feat(automations): reliable cron scheduling and webhook-driven push/CI triggers Automations were "wounded": cron schedules mishandled timezones and some never fired, and the GitHub push/CI trigger types were coming-soon stubs. Cron (fix): - The scheduler reduced every cron expression to a fixed interval derived only from a */N step in the minute/hour field, so any pinned schedule (0 9 * * *, 30 14 * * *, weekday ranges, day-of-month) never fired, and the configured timezone was parsed but never used. - Replace the interval approximation with robfig/cron/v3: shouldFire now computes the true next fire time in the configured timezone (DST-aware, defaulting to UTC for host-independence), anchored to last-evaluated or creation time. Dead interval-parsing helpers and shorthand constants removed. GitHub push + CI triggers (feat): - Implement push and CI-failure triggers end to end via the existing per-workspace GitHub App webhook plumbing (#1810): the HMAC-verified webhook service now parses push and completed check_run events, resolves the installation to its workspaces, and publishes normalized bus events. - A new automation subscriber matches enabled github_push / github_ci triggers by workspace + repo + branch/conclusion/check-name filters and fires them (deduped by commit SHA / check-run id). The old poll-based stub evaluators are removed; the PR poll path is unchanged. - Subscribe the App manifest to push/check_run and enable both trigger types in the registry (CI defaults to the failure conclusion). Docs: - Update docs/public/feature-status.md and automation-and-mcp.md so the cron and push/CI rows reflect shipped reality. Tests: regression tests pin the previously-never-firing cron shapes, timezone/DST behavior, and host-independence; backend tests cover the webhook parse->publish and subscriber match/fire/dedup flows for both push and CI-failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(automations): populate push.message and ci.url placeholders in fired trigger data Address PR review blockers: the default push and CI automation prompts reference {{push.message}} and {{ci.url}}, which the interpolator maps to data["message"] / data["html_url"] — but the webhook subscriber never populated those fields, so both always rendered empty. - Parse head_commit.message (push) and check_run.html_url (CI) in the GitHub webhook service and carry them on the published bus payloads. - Include "message" / "html_url" in the fired trigger data so the placeholders resolve; extend the webhook and subscriber tests to assert both flow through end to end. Non-blocking review cleanups: - Replace the misleading reuse of matchesAuthors for CI conclusions/check names with a clearly-named matchesFilterValue helper. - Drop dead context.Background() line and its unused import from the cron regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(automations): scope webhook triggers to active installs and per-branch dedup Address CodeRabbit review on the push/CI webhook triggers: - Security: resolveWorkspaceIDs now only returns connections whose status is active, so a suspended or revoked GitHub App installation can no longer fire push/CI automations. - Correctness: include the branch in the push dedup key (push:owner/name@branch@sha) so the same commit pushed to two matching branches fires once per branch instead of being suppressed. - Correctness: add an optional head-branch filter to github_ci triggers and gate CI matching on it, matching the push trigger's branch-scoping. - Stability: make the webhook subscriber's Start retryable — a subscription failure rolls back partial subscriptions and leaves the subscriber un-started so a later Start re-attempts, instead of permanently disabling push/CI processing. - Tests: cover multi-connection workspace resolution (dedup + inactive filtering), per-branch push dedup, and CI branch filtering. - Docs: note the CI branch filter and per-branch push dedup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(automations): cover webhook subscriber Start retry-after-failure path Add TestGitHubWebhookSubscriberStartRetryableAfterFailure with a minimal flaky bus whose second Subscribe fails, pinning the retry contract: a subscription failure rolls back the partial subscription and leaves the subscriber un-started, and a later Start re-attempts and succeeds. 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
|
49e00447a5 | fix: restore legacy GitHub launch compatibility (#1962) | ||
|
Carlos Florêncio
|
75abf779ed | feat(github): support per-workspace GitHub App registrations (#1810) | ||
|
Carlos Florêncio
|
3c09b1590a | fix: make remote repository entry reliable (#1936) | ||
|
José Almeida
|
7e4f2bb1e5 | feat(github): re-request dismissed reviews (#1921) | ||
|
Carlos Florêncio
|
bfc372b4ad | perf(github): speed up CI check feedback (#1882) | ||
|
José Almeida
|
7de788ceed |
feat: add cross-integration entity references (#1862)
* feat: add cross-integration entity references * fix(web): anchor entity reference menu to composer * fix: keep Kandev tasks under @ mentions * fix: address entity reference review findings * test(e2e): use option role for prompt menu * test(e2e): update passthrough mention roles * fix: preserve queued references and paginate Azure PRs |
||
|
Carlos Florêncio
|
5499eadf19 | fix(github): keep draft pull requests out of merge-ready state (#1851) | ||
|
João Salavisa
|
2a1f7c2edf |
feat: sync workflows from a configured github repo (#1721)
* feat: sync workflows from a configured github repo Per-workspace setting pointing at a GitHub repo directory of portable workflow export files. A background poller fetches it on an interval and creates, updates, or deletes the matching workflows; a force-sync endpoint bypasses the content-hash check. Synced workflows carry source/source_path provenance and coexist with manual ones; changes that cannot be applied safely (steps or workflows that still hold tasks, ambiguous step names, unparseable files) surface as warnings in the new settings section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): fill workflow sync form from a pasted github link A new Repository link field parses plain repo URLs, SSH remotes, and tree/blob links (which also carry branch and directory) into the structured owner/repo/branch/path fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): simplify workflow sync form to link, branch, and interval The repository link is now the only repo input: owner, repo, and directory resolve from the pasted URL (echoed as a summary under the field), and a stored config renders back into a canonical tree link via buildGitHubRepoUrl. Branch and poll interval stay individually editable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): collapse workflow sync into a button, dialog, and status card The Workflows settings page now shows only a GitHub Sync button by default; the link/branch/interval form lives in a dialog that closes on successful save or removal. Once configured, a compact status card (mirroring the GitHub connection-status card) shows the synced repo, branch, directory, last result, warnings, and a Sync now action. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): move github sync button into the workflows page header The GitHub Sync trigger now stacks under the Workspace settings button on the top right, and the extra separator around the sync area is gone — unconfigured workspaces show no sync chrome at all, configured ones show just the status card above the workflow list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): move github sync button next to the workflow actions GitHub Sync now sits in the Workflows section toolbar alongside Export All, Import, and Add Workflow; the status card renders inside the section above the workflow list. The toolbar moved to its own component to keep the page under the file-size limit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): separate sync status card from the workflow list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): align sync status card with the workflow list gutter Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: repair local edits to synced workflows on periodic sync Periodic syncs previously skipped applying when the repo content hash was unchanged, so local UI edits to a synced workflow were only reverted by a manual Sync now. Every sync now reconciles against the repo; the applier compares each step against its definition and only writes (and broadcasts) actual differences, so a no-drift sync stays silent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: make github-synced workflows read-only Workflows with source=github reject UI mutations: workflow rename, description, agent-profile, and delete return 409 from the task-domain handlers; step create/update/delete/reorder and template application are guarded in the workflow controller. The sync applier writes through the service/repo layer and stays exempt. The frontend disables the card's inputs, save/delete, and the step pipeline editor for synced workflows, with a tooltip pointing at the synced repository. Drift repair on sync remains as the backstop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: optional workflow sync polling and repo-derived branch picker A per-workspace auto-sync toggle (poll_enabled, default on) controls the background polling loop; when off the workspace only syncs via Sync now. The dialog's branch field becomes a select fed by the repository's actual branches (same loader the task-create dialog uses), falling back to free text until branches load. Existing dev databases gain the new column via an idempotent ALTER. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: release synced workflows when the sync config is removed Deleting a workspace's workflow sync configuration converts its GitHub-sourced workflows back to manual ownership (source=manual), so they become editable instead of being stranded read-only. Release runs before the config row is deleted so a failure keeps the config for retry. The settings page reloads after removal (and after syncs that changed workflows) so cards unlock without a manual refresh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web): drop branch selector and compact the auto-sync row The pasted link already carries the branch (main by default), so the dialog no longer needs a branch field — the resolved summary echoes it. The auto-sync toggle is now one compact row with the interval inline and a single short helper line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address PR review feedback Backend: fail-open read-only guard on workflow lookup errors (fixes MCP step handler CI failures); poller Stop holds its mutex through drain; per-workspace lock serializes syncs against config delete/replace; rollback workflow creation when provenance stamping fails; idiomatic duplicate-column migration via db.IsDuplicateColumnError; branch-name validation and a 30-day interval cap; generic 500s with logged detail; WS read-only rejections use CONFLICT with the real reason; e2e reset aborts when sync-config cleanup fails; test hygiene (t.Cleanup, SetMaxOpenConns). Frontend: malformed percent escapes parse as invalid instead of throwing; save disabled during initial load and for invalid intervals; Remove disabled while saving; stale async config responses discarded; delete-dialog confirms re-check read-only; focusable Synced badge; explicit button types; failed polls labeled 'last attempt'; unique warning list keys; test cleanup via afterEach. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: guard workflow sync test fake against concurrent poller access The poller invokes the fake applier from its own goroutine while the synctest test asserts on call counts; -race caught the unguarded reads. Also documents that the per-workspace lock is deliberately held across the GitHub fetch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Carlos Florêncio
|
0f266c319d | feat: link GitHub issues to tasks (#1672) (#1676) | ||
|
Carlos Florêncio
|
65ee3cac15 | fix: repair GitHub repository scope filters (#1641) | ||
|
Carlos Florêncio
|
164b80ac54 | feat(integrations): single workspace switcher with deep links and copy-config dialog (#1616) | ||
|
Carlos Florêncio
|
c3f713e494 | feat: scope GitHub integration settings by workspace (#1572) | ||
|
Carlos Florêncio
|
ef9807bd72 |
fix: coalesce ci auto-fix queue rounds (#1552)
* fix: coalesce ci auto-fix queue rounds * fix: centralize ci auto-fix round cap * fix: keep ci auto-fix state fresh * fix: refine ci auto-fix round status * fix: handle ci auto-fix exhaustion edge cases * fix: reduce ci auto-fix dispatch complexity * fix: refresh pending ci auto-fix before direct prompt * fix: harden ci auto-fix queue replacement --------- Co-authored-by: root <root@kandev.ts.net> |
||
|
Carlos Florêncio
|
5ed975ddbf | feat: redirect and toast when focused task is auto-deleted (#1544) | ||
|
Carlos Florêncio
|
5252a55199 | fix: improve pr ci automation reliability (#1508) | ||
|
Carlos Florêncio
|
b1e475b6bd | fix: paginate github check run fetching (#1506) | ||
|
Carlos Florêncio
|
365a3bdd24 | fix: recreate github review tasks after reset (#1492) | ||
|
José Almeida
|
23345c75d7 |
fix: show purple merged icon for recently merged PRs (#1483)
GitHub's /search/issues API returns merged PRs with state "closed" and a non-empty pull_request.merged_at, so the my-github search list rendered them with the red closed icon while the detail view (which checks merged_at) showed the correct purple merged icon. Plumb pull_request.merged_at through both the gh CLI and PAT search converters and promote those PRs to the "merged" state. |
||
|
Carlos Florêncio
|
4d19fb32f1 | feat: link tasks to github references (#1471) | ||
|
Carlos Florêncio
|
d58d9d03bb | fix: support issue links in remote task dialog (#1462) | ||
|
Carlos Florêncio
|
7ada858f17 | feat: add CI PR automation controls (#1446) | ||
|
Carlos Florêncio
|
aee1846cd5 | feat: remove nextjs production runtime (#1389) | ||
|
Carlos Florêncio
|
61acc82b74 | feat: add reset action for integration watches (#1381) | ||
|
Carlos Florêncio
|
817fbee0e5 | test(e2e): de-flake review-watch, port-forward, and pr-list specs (#1330) | ||
|
Carlos Florêncio
|
351db61e5b | fix: avoid stale sibling PR sync (#1302) | ||
|
João Salavisa
|
99d8d8f173 |
fix(github): handle UNIQUE collision in UpdatePRWatchBranchIfSearching (#1285)
* fix(github): handle UNIQUE collision in UpdatePRWatchBranchIfSearching
When a sibling PRWatch already owns the destination
(session_id, repository_id, branch) triple, the raw UPDATE tripped
the UNIQUE constraint and the change was logged as an error
("failed to update PR watch branch"). Wrap the update in a tx that
first checks for a sibling; if one exists, delete the source watch
(pr_number=0, no PR state to preserve) and cede branch ownership.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(github): close TOCTOU and cover sibling-has-PR collision case
Address PR review feedback on #1285:
- Catch SQLite "UNIQUE constraint failed" on the no-sibling UPDATE path
and treat it as the same collision case (Greptile P2 inline). A
concurrent CreatePRWatch that commits between our existence probe
and the UPDATE would otherwise re-trigger the bug this PR fixes.
- Switch the existence probe from `SELECT id` + `siblingID` dummy to
`SELECT 1` scanned into a typed `probe int` with an inline comment
(Claude inline readability nit).
- Extract `dropSourceAndCommit` helper so the probe-hit branch and the
TOCTOU fallback share one code path.
- Add `TestUpdatePRWatchBranchIfSearching_CollidesWithSiblingHasPR_DropsSource`
exercising the realistic race: sibling has already found its PR
(pr_number != 0) when source tries to migrate (Claude / Greptile P2 /
cubic P3 all flagged the gap). Asserts sibling row + PR number
survive untouched after source is dropped.
* test(github): cover missing-row no-op and reframe TOCTOU comment
Address Claude bot's follow-up suggestions on #1285:
- store.go: rewrite the UNIQUE-catch comment. The original wording
implied an in-process race, but SQLite's writer pool is
SetMaxOpenConns(1) so a concurrent in-process CreatePRWatch cannot
commit between probe and UPDATE. Reframe as defensive belt-and-
suspenders for an external SQLite writer / future pool changes.
- service_pr_watch_multi_branch_test.go: add
TestUpdatePRWatchBranchIfSearching_MissingRow_NoOp covering the
ErrNoRows -> tx.Commit() idempotency contract that the new
transaction code introduces. Locks the prior UPDATE-by-id semantics
of "0 rows affected, no error" for a nonexistent id.
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
José Almeida
|
4601dbc2d2 |
feat(tasks): search tasks by PR number in command panel (#1268)
* feat(tasks): search tasks by PR number in command panel Typing a PR number (e.g. 1243 or #1243) in the Cmd+K panel now surfaces the task that PR is associated with, alongside the existing title/description/repo search. The github store gains a workspace-scoped ListTaskIDsByPRNumber lookup over github_task_prs; the task service resolves it through an injected PRTaskResolver interface (github service wired in cmd/kandev) and merges PR matches into search results, deduped and respecting the archived/ephemeral/config filters. Keeping the lookup at the service layer avoids coupling the task repository to the github schema, so existing search queries and tests are untouched. * fix(tasks): scope PR-number search to page 1 and unfiltered queries Address review feedback on PR-number search: - Only augment results on the first page of an unscoped search. Running on every page re-prepended the PR match and truncated a real result off each subsequent page; running under a workflow/repository filter leaked tasks that don't satisfy the filter. - Add idx_github_task_prs_pr_number so the lookup is index-backed instead of a full scan (pr_number is the 3rd column of the existing UNIQUE constraint, unusable as a leading key). - Log instead of silently dropping a task when GetTask fails in the PR-match loop. * test(github): dedup PR-lookup test setup into shared newTestStore Add workspace_id to newTestStore's tasks table and drop the near-identical newTestStoreWithWorkspaces helper, per review feedback. * perf(tasks): batch-fetch PR-matched tasks instead of N+1 GetTask Add a repository GetTasksByIDs and use it in augmentWithPRMatches so PR-number search loads all matched tasks in one query instead of one GetTask per id, addressing the N+1 noted in review. Fetch+filter is extracted into a fetchPRMatchedTasks helper that preserves resolver order and dedup. |
||
|
Nicolas Lenepveu
|
72e3a49718 | fix(watcher): self-heal orphaned watchers when agent profile is soft-deleted (#1094) | ||
|
Carlos Florêncio
|
79d4f84bf2 | fix(backend): cache & singleflight live PR feedback fetches (#1237) | ||
|
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> |
||
|
José Almeida
|
36b5c682a9 |
fix(github): detect fork PRs by branch (#1182)
* fix(github): detect fork PRs by branch * fix(github): align fork branch detection paths * fix(github): skip duplicate branch PR matches * test(github): remove stale branch owner fixtures * test(github): remove stale PAT branch owner fixture * fix(github): surface branch decode errors |
||
|
Carlos Florêncio
|
d66d9e6f67 | feat: remote tab in task-create dialog — multi-row GitHub repo picker (#1116) | ||
|
Carlos Florêncio
|
96057e25f4 | fix(github): throttle PR branch-detection probes to stop log flood (#1135) | ||
|
João Salavisa
|
de92557f46 |
test(backend): wire goleak goroutine-leak detection across 8 packages (#1139)
* chore(backend): add go.uber.org/goleak test dependency Promotes the previously-indirect goleak v1.3.0 to a direct require so per-package TestMain calls can use it for goroutine leak detection. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(backend/lifecycle): drain workspace-stream backoff on Manager.Stop StreamManager.connectWorkspaceStream slept the full retry backoff in time.Sleep between failed attempts. SessionTraceContext is deliberately uncancellable (it carries a long-lived span), so if the Manager began shutting down mid-backoff the goroutine outlived Stop() and stranded until the timer fired. Thread Manager.stopCh through the StreamManager constructor and add a sleepOrStop helper that selects between the timer and the shutdown signal. ReconnectAll's startup sleep gets the same treatment. TestConnectWorkspaceStream_BackoffDrainsOnStop closes stopCh while a failing-port retry is in flight and asserts the goroutine exits within 500ms — fails without the fix, passes with it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(backend/github): close sqlite DB in newTestStore to drain connectionOpener newTestStore opened *sqlx.DB via db.OpenSQLite but never registered a Close on cleanup, leaving database/sql.(*DB).connectionOpener goroutines parked across test boundaries. Add t.Cleanup(sqlxDB.Close); goleak's per-package TestMain (added in a follow-up commit) is the regression check. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(backend): wire goleak.VerifyTestMain across leak-prone packages Per-package TestMain in: - internal/integrations/healthpoll (auth-health ticker loop) - internal/jira, internal/linear (auth + issue-watch loops) - internal/github (PR / review / issue poller loops) - internal/gateway/websocket (per-conn pumps + terminal bridge) - internal/orchestrator (detached event-handler goroutines) - internal/agent/runtime/lifecycle (remote-status loop, StreamManager helpers; ignores OTel batch-span processor and OTLP retry helper — initialised early in the process and only exit on shutdown) - internal/agentctl/server/process (extends the existing fixture-binary TestMain so subprocess + PTY pump leaks surface there) Catches regressions where a Start/Stop forgets to drain on WaitGroup, or a detached goroutine forgets to honour ctx/stopCh cancellation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(backend): document goroutine ownership and leak-testing convention New "Goroutine ownership and leak testing" subsection under Conventions in apps/backend/AGENTS.md (CLAUDE.md symlinks to it). Covers: - Lifecycle: Start/Stop + WaitGroup, both idempotent. - Cancellation: every long wait selects on ctx.Done() or stopCh; never time.Sleep in retry loops (reference lifecycle.StreamManager.sleepOrStop). - Detached helpers: must respect the owning type's shutdown signal. - Leak testing: new packages spawning goroutines add goleak.VerifyTestMain; IgnoreTopFunction is reserved for genuine third-party background goroutines and must carry a comment explaining the suppression. Lists the packages currently instrumented so future contributors can mirror the pattern. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(backend/lifecycle): drop dead OTel IgnoreTopFunction from goleak setup Review follow-up. agentctl/tracing.initTracing only spawns the OTel SDK provider when OTEL_EXPORTER_OTLP_ENDPOINT is set; in tests it isn't, so the noop provider is used and no batchSpanProcessor / OTLP retry goroutines ever exist. The IgnoreTopFunction calls were dead suppressions and the comment ("called by NewManager via the agent execution trace path") misdescribed when the SDK init runs. Drop the ignores and correct AGENTS.md to stop citing OTel as the canonical suppression example. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(backend/lifecycle): drop unneeded 50ms sleep from backoff-drain test QA follow-up. The 50ms time.Sleep before close(stopCh) in TestConnectWorkspaceStream_BackoffDrainsOnStop was meant to ensure the goroutine had entered the backoff sleep before shutdown — but that ordering doesn't affect the test outcome: - With the fix: sleepOrStop sees stopCh already closed and returns false on the first call, so the goroutine drains within one attempt regardless of when stopCh closes. - Without the fix: time.Sleep ignores stopCh, the goroutine burns its full 5-retry backoff (~31s), and the 500ms timeout fires. Removing the sleep makes the test ~12x faster (15ms vs 200ms total) and removes the only time.Sleep-based wait from the new tests, aligning with the codebase's "channel-based sync, no sleep waits" convention. Verified by 50x -race and 5x revert-and-fail. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(backend/github): drain HTTP idle conns before goleak inspects CI surfaced a leaked goroutine on the Linux runner that local runs missed: a single net/http.(*http2ClientConn).readLoop parked on a TLS read, created by net/http.(*http2Transport).newClientConn. The package's anonymousHTTPClient and PATClient's rewriteTransport both route through http.DefaultTransport's connection pool, so an idle HTTP/2 connection in that pool keeps the reader goroutine alive past m.Run() — invisible on sandboxed local runs where outbound network is blocked. Replace github/goleak_test.go's plain VerifyTestMain with an expanded TestMain that drains http.DefaultTransport's idle connections before goleak.Find runs. This is the stdlib's documented teardown for the connection pool, keeps the assertion strict (no IgnoreTopFunction), and doesn't leak into production code. Other in-scope packages don't need this — they don't use http.DefaultTransport for HTTPS calls. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(backend): drain ws.Done on stopCh; always run goleak in github tests - lifecycle/streams.go: replace bare <-ws.Done() with a select that also watches sm.stopCh so a connected goroutine drains when Manager.Stop() fires before the remote WS closes. A nil stopCh (isolated tests) blocks the nil channel case forever, degrading to plain <-ws.Done() semantics. - github/goleak_test.go: remove the exitCode==0 guard around goleak.Find so leaks are always surfaced even when a test has already failed; original exit code is preserved (leaks only promote it to 1). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backend/lifecycle): close ws on stopCh so agentctl WS loops drain cubic flagged that the previous shutdown path observed stopCh but never called ws.Close(), so the underlying WebSocket and its agentctl-side read/write goroutines (workspace_stream.go:67-68) kept running past Manager.Stop(). ClearWorkspaceStream only removes the WS from the execution's tracking — it doesn't close the connection. Call ws.Close() on the stopCh path. ws.Close is idempotent via closeOnce, so it's safe even if the remote end already closed. TestConnectWorkspaceStream_ClosesWSOnStop connects to a mock workspace server that stays open until the client closes its side; after stopCh fires, the test asserts ws.Done() closes within 500ms — which is only true if connectWorkspaceStream invoked ws.Close(). Verified to fail without the fix and pass with it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(backend/lifecycle): use channel sync instead of sleep polling in ws-close test cubic flagged that TestConnectWorkspaceStream_ClosesWSOnStop polled execution.GetWorkspaceStream() in a 500ms sleep loop, which is brittle under CI load. connectWorkspaceStream already accepts a ready channel that closes after SetWorkspaceStream — pass it in and wait on the channel instead. synctest isn't viable here (real network I/O), so channel-based sync is the testing convention per AGENTS.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
João Salavisa
|
651d66a0cf |
refactor(github): split service.go into domain subfiles (#1053)
Splits the 2,842-line apps/backend/internal/github/service.go monolith into nine focused files, all under the 800-line revive threshold. No public API changes, no behavior changes — Service struct, constructor, and every method signature preserved verbatim. Files: errors.go (sentinels), service.go (struct + constructor + getters), service_tokens.go, service_pr.go, service_pr_watch.go, service_reviews.go, service_issues.go, service_cleanup.go, service_pr_status.go. Only semantic-equivalent code rewrites: CheckPRWatch hasNew collapsed to a single || expression; three narrow //nolint directives for pre-existing complexity reflagged by golangci's new: true mode after the move. Rebased onto current main after PR #1057 (sentinel extraction) landed — took its errors.go (now 4 sentinels incl. ErrInvalidToken) and ported the %w wraps for ConfigureToken and AssociateExistingPRByURL into the split files. |
||
|
João Salavisa
|
dcfec0d23c |
refactor(backend): classify errors via sentinels instead of substring matches (#1057)
* refactor(backend): classify errors via sentinels instead of substring matches
Introduce package-local sentinel errors in the github, task repo, agent
lifecycle, and office/skills packages. Producers wrap with fmt.Errorf("%w: ...")
so callers can classify via errors.Is/errors.As instead of brittle
strings.Contains(err.Error(), ...) checks that break on library upgrades.
Sentinels added (or relocated to errors.go):
- github: ErrInvalidToken (new); ErrInvalidPRURL, ErrTaskNotFound, ErrSelfApprove
moved from service.go.
- task/repository/sqlite: ErrTaskNotFound, ErrOfficeSessionRaceConflict.
- lifecycle: ErrAgentAlreadyRunning, ErrAgentReported.
- office/skills: ErrSkillNotFound, ErrSkillFileNotFound.
Consumers updated to errors.Is/errors.As across orchestrator, task/handlers,
github, and office/skills. AssociateExistingPRByURL switches %v -> %w so the
inner parsePRURL error stays in the chain. process_handlers now uses
errors.Is(err, syscall.ECONNREFUSED) and lifecycle.ErrNoExecutionForSession
plus a small isTimeoutError helper backed by errors.As.
A new taskDeleterAdapter in cmd/kandev/turn_adapters.go translates
taskrepo.ErrTaskNotFound to github.ErrTaskNotFound at the wiring boundary,
avoiding a layering violation between the github and task packages.
The one remaining substring check, in CreateTaskSession, is encapsulated
inside the repo and converts the SQLite UNIQUE-violation message into the
typed ErrOfficeSessionRaceConflict sentinel so callers never see the driver
string.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test(backend): add TestErrorsAreClassifiable for refactored packages
Pin the new sentinel-based contract for each refactored package. Each test
exercises the producer paths and asserts errors.Is reaches the sentinel
through the wrap chain, and (where applicable) confirms untyped lookalike
strings no longer classify.
- github: isTaskNotFound, AssociateExistingPRByURL/ErrInvalidPRURL,
ConfigureToken/ErrInvalidToken (using a noop secret manager).
- task/repository/sqlite: GetTask, DeleteTask, UpdateTaskState wrap
ErrTaskNotFound.
- task/handlers: isNotFound + isAgentReportedError follow the typed chain.
- orchestrator: each isXError classifies exact + wrapped sentinels, rejects
untyped lookalikes.
- office/skills: readUserHomeSkillInventoryFile wraps ErrSkillFileNotFound;
ErrSkillNotFound survives fmt.Errorf("%w", ...).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test(github): drop network-hitting subtest from errors_test
The "ConfigureToken wraps ErrInvalidToken when validation fails" subtest
constructed a real PATClient and called GetAuthenticatedUser, which makes
a live HTTP request to api.github.com. That's flaky in CI / offline dev
and violates the unit-test boundary (caught in review).
Replace with a sentinel-survives-wrap-pattern check that locks in the
exact %w/%w shape used by ConfigureToken, plus an assertion that the
inner cause stays reachable through errors.Is. The full ConfigureToken
flow stays covered by service_token_test.go and integration tests.
Remove the now-unused noopSecretManager stub type.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task): restore "timed out" UX for plain-fmt.Errorf timeout producers
The original createPromptErrorMessage rendered the friendly "Request timed
out…" hint via strings.Contains(err.Error(), "timeout"). The refactor
replaced it with errors.Is(context.DeadlineExceeded) + a net.Error
Timeout() helper — strictly typed. But several upstream producers along
the prompt path (service.WaitForSessionReady, agent-stream connect waits
in lifecycle/session.go and manager_interaction.go, agentctl health waits
in agentctl/client.go and launcher.go, the orchestrator's own
waitForSessionReady) return plain fmt.Errorf("timeout …") with no typed
Timeout() method, so the typed check missed them and the user saw the
generic "Failed to send message to agent" instead.
Add a substring fallback inside isTimeoutError as a UX safety net while
keeping the primary typed paths intact. Lock the contract with table
tests covering all three classes (net.Error timeout, plain string,
context.DeadlineExceeded) so any future trim of the fallback breaks
loudly. Migrating each upstream producer to a typed sentinel is tracked
as out-of-scope follow-up.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test(backend): tighten classification tests to exercise typed-sentinel contracts
Addresses bot-review feedback on PR #1057:
- task/handlers/errors_test.go: cover all four not-found sentinels
(taskrepo.ErrTaskNotFound, service.ErrDocumentNotFound,
service.ErrTaskPlanNotFound, service.ErrRevisionNotFound) in
TestErrorsAreClassifiable via a table-driven loop, instead of only
one. Drop the spurious Deadline() *time.Time method on timeoutNetErr
— it isn't part of net.Error and the existing `var _ net.Error =
timeoutNetErr{}` line already pins the interface.
- orchestrator/scheduler/scheduler_test.go: testTaskRepository was
still returning the local scheduler.ErrTaskNotFound, which doesn't
satisfy the production processTasks check (now errors.Is on
taskrepo.ErrTaskNotFound). Mirror the real sqlite repo's wrap so the
mock and prod paths agree. The local scheduler.ErrTaskNotFound is
truly dead after that, so drop it from scheduler.go.
- office/skills: the wrap-pattern subtest in errors_test.go was only
testing fmt.Errorf, not production code. Move the contract assertion
to service_test.go (external skills_test package) where it drives
the real GetSkillFromConfig lookup path against an empty in-memory
repository.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* refactor(backend): drop redundant context.DeadlineExceeded check and fix doc grammar
Addresses bot-review feedback on PR #1057:
- task/handlers/message_handlers.go::createPromptErrorMessage: the
explicit errors.Is(promptErr, context.DeadlineExceeded) is redundant
with isTimeoutError, which already matches via errors.As against
Timeout() — context.DeadlineExceeded implements Timeout()==true.
Inline comment documents the consolidation.
- github/errors.go: fix grammar in the ErrTaskNotFound doc comment
("is the sentinel cleanup paths check for to distinguish" →
"is the sentinel that cleanup paths check to distinguish").
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
João Salavisa
|
b4a696a5e9 |
fix(github): hide approve PR button on own PRs (#1017)
* fix(github): hide approve PR button on own PRs and reject self-approval server-side The frontend's existing self-author check failed open when the current GitHub user hadn't loaded yet — the PR panel didn't call useGitHubStatus, so the green Approve button flashed on every PR (including your own) during the brief window before /api/v1/github/status resolved. Fail closed when the user is unknown, fetch the status from the panel, and add a typed ErrSelfApprove guard in Service.SubmitReview so any path that bypasses the visibility check gets a clean 422 instead of an opaque upstream error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(github): cover controller's self-approve 422 mapping Address PR review feedback: add TestHttpSubmitReview_SelfApproveReturns422 to exercise the controller → service → 422 path end-to-end so a future refactor can't silently reclassify ErrSelfApprove as a generic 500. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
João Salavisa
|
0f1081d423 |
feat(share): add public share links via GitHub Gists (#995)
* feat(share): add public share links via GitHub Gists Adds a Share button to the chat panel header that publishes the current session as a secret GitHub Gist on the user's account. The share link routes through gistpreview.github.io to render share.html as a styled kandev-themed page; snapshot.json + README.md ride along for machine-readable + fallback views. Snapshots run a redaction pass (absolute paths, common credential shapes, .env contents, shell env vars) and strip <kandev-system> prompt blocks via the shared sysprompt helper. Message filtering whitelists user prose + tool_* types so internal noise never leaks into shares. Spec at docs/specs/public-share-links/spec.md. * fix(share): coerce pendingClarification to bool for ClarificationSection A late refactor extracted ClarificationSection with `pendingClarification: boolean` on its props, but the call site passes the actual Message|null value. CI's tsc build catches the mismatch; runtime was unaffected because the section only uses the value in a truthiness check. Coerce with Boolean() at the call site to keep the prop's intent narrow. * fix(share): address PR #995 review feedback Bugs (flagged by Greptile, Cubic, Claude): - abs-path redaction split on `root + "/"` so sibling dirs like /home/user/proj2 don't get corrupted when workspaceRoot is /home/user/proj; regression test added. - GHClient.DeleteGist now wraps "HTTP 404" stderr as *GitHubAPIError so share.IsAlreadyGone matches consistently with the PAT client; this restores the "gist already revoked upstream" soft-success path for CLI-authenticated users. - SnapshotSizeBytes is now measured from the same MarshalIndent payload the gist backend uploads, so the size shown in the UI matches the actual gist bytes (was ~20-40% underreported). Polish (flagged by CodeRabbit, Cubic): - Tool name + tool output now HTML-escaped; tool call args and output render via <pre><code> instead of triple-backtick fences so payloads containing ``` can't break out of the code block. - share-list.tsx: clear `busy` via finally and break error layout out of the justify-between row. - use-shares.ts: convert UseSharesResult to interface (repo TS rules); reset error+isLoading when ids are missing. - use-shares.test.tsx: wait on the actual async outcome (shares populated, listShares called) instead of isLoading flipping false. - provider.go: replace `repo.Close` cleanup with an explicit noop closure so the comment ("Cleanup is a no-op") and the impl match. - spec.md: aligned the shareability rule across What/Failure/Scenarios/ Out-of-scope — any non-prehistory state can be shared; added language tags to fenced code blocks for markdownlint. Skipped intentionally (reply on each thread): - HTML-escape every metadata badge: the values are controlled enum strings from kandev's own schema, not user input. Defense-in-depth noise. Tool name (which can come from agent runtime) IS escaped. - useShares request-token guard against stale responses: the dialog owns the hook lifecycle and opens against a single (taskId, sessionId) — the race the comment describes can't occur here. * fix(share): redact paths for every worktree, not just the first A single TaskSession can carry multiple worktrees when the task spans multiple repos. The redactor previously only saw the first worktree path, so absolute paths from secondary worktrees leaked into public shares instead of being rewritten to repo-relative form. NewRedactor is now variadic and applies the abs-path rule against every root it receives, sorted longest-first so a root that is a prefix of another (e.g. /workspace vs /workspace/repo1) gets matched by the more specific one first. workspaceRootFor in builder.go becomes workspaceRootsFor and emits every WorktreePath plus the WorkspacePath fallback. Flagged by Cubic on PR #995. * fix(share): address PR #995 review round 3 Bugs (flagged by Cubic + Claude on gh_client.go:714): - DeleteGist now calls the existing isNotFoundErr helper instead of an inline strings.Contains(err, "HTTP 404"). The helper also covers "404 Not Found" and "status: 404", which some gh versions emit, so the share service's "gist already revoked upstream" soft-success path now fires consistently across CLI auth users. Bugs (flagged by Claude on builder.go:220): - toolCallBlock used to read metadata["tool_name"] and metadata["args"] but production never writes those keys; the typed payload lives under metadata["normalized"] (a *streams.NormalizedPayload) and the tool kind is on Message.Type. Switched to deriving ToolName from strings.TrimPrefix(m.Type, "tool_") (gives "read", "execute", "edit", …) and reading args from metadata["normalized"], with backward-compat fallback to the older "tool_name"/"args" keys for the debug-fixture replay path. New TestBuildSnapshot_ToolCall_ProductionMetadataShape exercises the real session shape. Bugs (flagged by CodeRabbit on redaction.go:110): - Bare workspace-root occurrences (pwd output, end-of-sentence, quoted paths) used to leak because the matcher only handled root + "/". Switched to a precompiled regex per root — regexp.QuoteMeta(root) + `(?:/|\b)` — so both `root/...` and standalone `root` get stripped, while sibling dirs like /workspace2 still don't match. Test TestRedactor_AbsPath_StandaloneRootStripped covers the bare-root variants. Doc (flagged by Claude on spec.md): - Two malformed fence closers (`go and `text on closing fences) corrected to plain ``` so the file is markdownlint-clean. * fix(share): narrow abs-path boundary to avoid sibling-prefix corruption Round 4 review (Claude + CodeRabbit) caught a regression in the regex introduced by 436cb66: `(?:/|\b)` used `\b` (word boundary) to catch bare-root cases, but `\b` fires on any word↔non-word transition. So a sibling path like `/workspace.bak`, `/workspace-old`, or `/workspace-other` would false-match the `/workspace` root (since e→. and e→- are word boundaries), stripping it and leaving a corrupted `.bak` / `-old/file.ts` in the share. Replaced the matcher with `(/|\s|$)`: path separator, whitespace, or end-of-string only — never punctuation or quote chars. The boundary is captured and re-emitted via ReplaceAllStringFunc (or dropped, in the `/` case) so prose loses only the root, not the surrounding space. Added TestRedactor_AbsPath_SiblingsWithPunctuationNotCorrupted covering `.bak`, `-old`, `_alt` siblings; the digit-sibling test still passes. The quoted-bare-root case (`"/workspace"`) no longer matches and was dropped from the standalone-root test — contrived enough that a small leak there is preferable to corrupting realistic sibling paths. * refactor(share): move share button into chat status bar Replaces the dedicated PanelHeaderBarSplit above the message list with a slot in ChatStatusBar (the existing row holding TodoIndicator and PRStatusChip just above the chat input). The button anchors right via ml-auto; when the "proceed to next step" affordance is present it takes the right anchor instead and the share button stays inline. Also tightens the iconOnly variant in ShareButton to a ghost style that matches the muted-foreground chips in the status bar, and exports shareableSessionStateClient from share-button.tsx so the gating logic lives next to the entry point it controls. Net -23 lines in task-chat-panel.tsx: the extra header bar and the local gate helper are gone. * docs(share): broaden tooltip + add share/ to CLAUDE.md tree The share tooltip read "Share this completed task with a public link", but shareableSessionStateClient allows RUNNING / IDLE / WAITING_FOR_INPUT / FAILED / CANCELLED too. A user hovering mid-run saw misleading copy; reworded to "Share this task conversation as a public link" which matches the actual gate. Also added the missing internal/task/share/ entry to the task/ subtree in CLAUDE.md so the new package shows up in the repo map — flagged across multiple rounds. * docs: add internal/task/share/ to repo map Carryover from the share PR — AGENTS.md (which CLAUDE.md symlinks to) was missing the share/ subpackage under task/. Adding it now so the repo map matches what's checked in. --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
João Salavisa
|
b8a09bc5ea |
fix(github): pick allowed merge method so squash-only repos stop 405ing (#999)
* fix(github): pick allowed merge method so squash-only repos stop 405ing The merge button sent an empty merge_method to GitHub, which defaults to "merge" regardless of repo settings — squash-only / rebase-only repos returned 405 "Merge commits are not allowed on this repository". The backend now resolves an empty method to the first allowed one via a new GetRepoMergeMethods endpoint (5-min cache), and the frontend renders a GitHub-style split button so users can pick among the allowed methods. * fix(github): address PR #999 review feedback - pr-merge-button: extract repo-methods fetch to useRepoMergeMethods hook (Claude bot); hide button while methods are loading so the primary label doesn't flicker (Claude bot); drop e.preventDefault on dropdown items so Radix closes the menu on selection (Greptile P1). - gh_client / pat_client: treat missing allow_* fields as false instead of true so a permission-gated response can't pick a disallowed merge method (cubic P2). - service.MergePR: only overwrite mergeMethod when pickDefaultMergeMethod returns non-empty, so a degenerate all-false config falls through to GitHub's default with the real error (Claude bot). * fix(github): don't hide merge button on lookup failure Round-2 PR feedback (Claude, Greptile, CodeRabbit, cubic): the `if (!methods) return null` guard added in the previous round to avoid label flicker turned a transient repo-methods lookup failure into a permanent UI lockout — the button would stay hidden until unmount/remount. - pr-merge-button: revert the null-guard; treat null as "use backend resolver", render the plain primary button without a dropdown. - use-repo-merge-methods: add a 5-min frontend TTL to match the backend cache so admin merge-strategy flips are visible without a page reload. Catch block now triggers a rerender, and co-waiting consumers attach both fulfilled/rejected handlers to avoid silent rejection swallowing. * test(github): cover useRepoMergeMethods + silence rejection warning Round-3 PR feedback (Claude): - Swallow rejections in the originating fetch chain instead of rethrowing, so the first consumer's promise never escapes the hook as an unhandled rejection. Coalesced consumers already re-render to null, which is the same state a fresh "not loaded" call would show. - Add use-repo-merge-methods.test.ts covering: null-on-missing-params, loading then resolution, cache hit on remount, concurrent-fetch coalescing, retry-on-failure (no cache poisoning, no unhandled rejection), and TTL-expiry refetch via fake timers. * test(github): cover httpGetRepoMergeMethods error branches Round-4 PR feedback (Claude): the new endpoint had only happy-path coverage. Add tests matching httpMergePR's existing shape: - nil client → 503 (ErrNoClient) - GitHubAPIError{404} → 404 - generic upstream error → 500 --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
José Almeida
|
1f264adbc4 | fix(github): drain piled-up PR/issue review tasks via cleanup policy + manual sweep (#929) | ||
|
João Salavisa
|
3d860290da |
feat(github): add merge button when PR is ready to merge (#979)
* feat(github): add merge button when PR is ready to merge Surfaces a one-click merge action in the PR detail panel header and as a compact green pill in the PR topbar hover popover, gated by the existing isPRReadyToMerge predicate (open, CI success, mergeable_state=clean, and either approved or no review process). Backend adds MergePR to the Client interface, implements it across PAT/gh/Noop/Mock clients, exposes PUT /api/v1/github/prs/:owner/:repo/:number/merge, and maps GitHub 405/409 to HTTP 409 so the UI can show a clear error. * fix(github): address PR review feedback on merge endpoint - Map gh CLI 404/403/405/409 stderr to *GitHubAPIError via a new ghMergeStatusCode helper, so merge conflicts surface as HTTP 409 for gh CLI users (previously only PAT users got the right status). - Service.MergePR returns ErrNoClient when unconfigured; httpMergePR translates that to HTTP 503 with github_not_configured, matching httpListRepoBranches. - httpMergePR distinguishes empty body (io.EOF, fall through to default merge method) from malformed JSON (HTTP 400 invalid payload). - PRMergeButton tracks a local merged flag so the button stays hidden across the async refresh window after a successful merge. * test(github): cover merge helpers and extract PRMergeButton - TestGhMergeStatusCode locks in the gh CLI stderr → status-code parser for nil, unrelated errors, the HTTP / status: / bare-phrase forms of 404/403/405/409, and a 500 non-match guard. - Broaden ghMergeStatusCode to also match "405 Method Not Allowed" / "409 Conflict" phrase forms so the merge endpoint's 409 mapping does not depend on the gh CLI keeping its current "HTTP <code>" prefix. - TestHttpMergePR_MalformedJSON covers the new non-EOF bind-error branch (truncated body → 400). TestHttpMergePR_NoClient_Returns503 covers the ErrNoClient → 503 path on Service.MergePR. - Document the post/put error-type divergence on PATClient.post so a future caller does not silently lose per-status mapping. - Move PRMergeButton out of pr-shared.tsx into its own file, and reset the local "merged" flag when taskPR.id changes so an instance that re-renders for a different PR does not stay hidden. --------- Co-authored-by: Kandev Agent <agent@kandev.dev> |
||
|
Luan Martins
|
a852be3c33 |
feat(web): show task linkage on GitHub PR list (#952)
* feat(web): show task linkage on GitHub PR list
Add a compact indicator next to each PR showing whether a task already
exists. When tasks are linked, show a clickable button with the task
title and a count badge for multiple tasks. When no task exists, show
'No task created yet'.
- Add usePRKeyToTasks hook that maps PR keys to task associations
- Add PRRowTaskIndicator component for the per-PR task UI
- Wire the data through github-page-client into PRList
- Add tests for the indicator and hook utility
* feat(web): improve PR task indicator with task titles and multi-task dropdown
- Show actual task title instead of PR title in single-task case
- Replace count badge with dropdown menu for multiple linked tasks
- Switch icon from GitPullRequest to Checklist
- Load all workflow snapshots on GitHub page to hydrate task titles
- Fix test to use unique task_id per fixture
* feat(web): neutralize PR task indicator color and add tooltip
The checklist icon previously mirrored PR aggregate status, which duplicated
information already shown on the PR row and misleadingly suggested it
reflected task state. Drop the color and add a tooltip surfacing the actual
task title and workflow step.
Adds e2e coverage for the GitHub PR list task indicator: empty badge,
single-task button, and tooltip content.
* fix(web): navigate to task page when clicking PR task indicator
replaceTaskUrl only mutates history.state, which sets active task in the
store but does not trigger a Next.js route change. From /github, this left
the URL pointing to /t/<id> while the page still rendered the GitHub list.
Use router.push so the click actually navigates to the task.
* test(web): cover PR task indicator click navigation
Extends the GitHub PR list task indicator spec with a click assertion
verifying the URL changes to /t/<taskId> and the PR list unmounts.
Regression guard for the replaceTaskUrl → router.push fix.
* fix(github): link task to PR at creation time, not via branch poller
Tasks started from the GitHub PR list "+ Task" button were left without a
github_task_prs row when the worktree branch did not match the PR's head
branch on GitHub. This happened for PR-review-style tasks, which check out
synthetic branches (e.g. feature/review-...-h92) that don't exist as PR
heads. The poller's FindPRByBranch could never resolve them, so only the
first task whose branch happened to match got linked — every other task
created from the same PR was missing from the indicator.
The PR is already known at "+ Task" time, so persist the linkage
immediately rather than waiting on branch-based discovery:
- AssociateExistingPRByURL: new service method that creates the
github_task_prs row from a PR URL without creating a PR watch. The
watch is still created later at agent start via ensureSessionPRWatch.
- POST /api/v1/github/task-prs/associate: HTTP endpoint exposing it.
- QuickTaskLauncher.handleSuccess: after task creation, fires a
best-effort associate call with the task's repository_id and PR URL.
Failures fall back to the legacy poller path silently.
Backend test covers two distinct tasks linking to the same PR.
E2e adds a PR with two tasks and asserts the multi-task indicator
("Tasks 2") renders alongside the single-task and empty cases.
* refactor(github): make task-PR create endpoint RESTful
Rename POST /api/v1/github/task-prs/associate to POST /api/v1/github/task-prs.
The action-named "/associate" was RPC-styled; the resource collection is the
right URL for creation alongside the existing GET. Frontend client renamed
associateTaskPR → createTaskPR to match.
* fix(github): address PR review findings
- Map invalid PR URL to HTTP 400 (was 500). New ErrInvalidPRURL sentinel
lets the controller distinguish parse failures from genuine server
errors. Other failure paths still return 500.
- Prefer dialog-time repositoryId in pickRepositoryIdForPR. Multi-repo
tasks with shared branch names were ambiguous under pure branch
matching; the matched repo id captured by the launcher dialog wins
when present.
- Cover usePRKeyToTasks (not only the prKey util) with hook tests:
empty input, multi-task grouping, distinct keys, and the non-array
defensive branch.
- Lock createTaskPR request semantics: method/body now win over a
caller's options.init so the POST JSON shape can't be overridden.
* fix(web): use TaskPR row id for dropdown key
Multi-repo tasks can produce more than one TaskPR row for the same PR
(one row per repository_id), so keying the multi-task dropdown by
task_id risked a duplicate-key warning and unstable reconciliation.
Switch to the row id, which is unique per row.
|
||
|
Carlos Florêncio
|
0b2085688b | feat(office): autonomous agent management layer (#914) | ||
|
José Almeida
|
14c19ea538 |
feat(web): add CI hover popover on PR top-bar button (#846)
* feat(github): plumb CI popover fields + branch protection cache
Adds RequiredReviews/UnresolvedReviewThreads/ChecksTotal/ChecksPassing
to TaskPR + PRStatus. Extends the GraphQL PR query with reviewThreads
to populate UnresolvedReviewThreads on every poll. New branch_protection.go
holds an in-memory TTL cache + BranchProtectionFetcher interface;
PATClient/GHClient implement REST/CLI lookups so SyncTaskPR can resolve
required_approving_review_count for the PR's base branch. SyncTaskPR
preserves existing counts when a lightweight (GraphQL) sync returns 0
so the popover doesn't flap during the polling cycle.
* feat(github): mock controller hooks for CI popover e2e
Extends associateTaskPR to accept review/check counts, required_reviews,
and unresolved_review_threads, and switches to ReplaceTaskPR so re-POSTing
the same task drives the popover live-refresh path. Adds POST /pr-feedback
to seed PRFeedback (checks/reviews/comments) keyed by a deterministic
synthetic head_sha, and PUT /auth-health to flip the mock client's
authenticated flag for the "Reconnect GitHub" branch. Mock client gains
ReplaceCheckRuns/ReplaceReviews/ReplaceComments helpers and an
authenticated field that defaults to true.
* feat(web): CI hover popover on PR top-bar button
Hovering the PR top-bar button now reveals a popover with
Passed/In-Progress/Failed counts, a workflow-rollup list under the
non-empty buckets (with "open on GitHub" + "+ ctx" affordances on
failed rows), a review-state row ("Approved 1 / 2 required"), and
unresolved-comments count. Click on the button keeps the existing
"open PR detail panel" behavior so shipped tests stay green.
PRFeedback is lazy-fetched on hover-open via a new SWR-style hook
backed by a 20-entry LRU cache slice; the popover refetches whenever
the underlying TaskPR's updated_at changes (the WS handler already
applies poll deltas). Bucket logic + workflow grouping live in a
small util with vitest coverage. The aggregate-popover variant for
multi-PR tasks ships but the multi-PR trigger preserves its existing
DropdownMenu so per-row click → addPRPanel still works.
* test(e2e): pr-topbar-popover scenarios A–F covering CI popover
11 specs covering: count rows + zero-bucket hiding, workflow rollup
with (N/M passed) badge under Failed, review row + unresolved comments,
hover open + leave close, failed-row open/+ctx affordances, header
external link, "No checks have started" empty state, "Reconnect GitHub"
auth-lost branch, and live count updates on a re-associate.
Adds api-client helpers (mockGitHubSeedPRFeedback, mockGitHubSetAuthHealth,
extended mockGitHubAssociateTaskPR) and SessionPage popover accessors.
* fix(github): address bot review comments on CI popover
- branch_protection.go: evict stale entries on read so the cache map
cannot grow unbounded over a long-running deployment; rewrite the
comment to drop the inaccurate singleflight claim (no dedup logic
exists, concurrent misses race harmlessly to set()).
- gh_client.go: replace fragile strings.Contains "HTTP 404" with an
isNotFoundErr helper that tolerates the formats current and future
gh CLI versions use ("HTTP 404", "404 Not Found", "status: 404").
- graphql.go: extend reviewThreads with totalCount and attribute the
un-fetched tail to the unresolved bucket so PRs with >100 threads
don't silently undercount in the popover.
- pr-ci-popover.tsx: drop the dead "icon" initializer in PRReviewRow
(every branch overwrites it). Memoize useAddCheckToContext so
PRWorkflowRow's reference equality stays stable across renders.
- pr-topbar-button.tsx: collapse the duplicate JSDoc on
usePopoverInteractions.
* fix(github): address remaining bot review comments + tests
Real correctness fixes:
- Replace the "0/0 means counts omitted" heuristic with an explicit
PRStatus.ChecksPopulated flag so the rich REST sync can clear stale
totals when checks are removed from a PR upstream.
- Refresh tp.BaseBranch from status.PR on each SyncTaskPR so a
retargeted PR's branch-protection lookup keys off the new branch.
Persist base_branch in UpdateTaskPR.
- Cache 403 from /branches/.../protection (token lacks Administration:
Read scope) the same way as 404 — otherwise every poller tick burns
rate-limit quota retrying. Both PATClient and GHClient.
- usePRCIPopover: refetch on every popover-open transition (not just
on updated_at change), via a wasEnabledRef gate. Restores the
intended SWR semantics that the lastSyncedRef-only guard suppressed.
- groupChecksByWorkflow: prefer the URL of a job matching the group's
bucket so clicking a "failed" workflow row navigates to a failing
job, not an earlier passing one.
Tests:
- branch_protection_test.go: cache hit/miss/TTL/eviction, error
not cached, no-rule cached, client-without-fetcher path.
- pat_client_test.go: TestPATClient_FetchBranchProtection table-test
for 200/200-with-null/404/403/500.
- service_test.go: ChecksPopulated preserve-on-zero, accept real
zero, base-branch retarget, change-publish for the new fields.
* refactor(github): trim associateTaskPR + drop unreachable PRAggregatePopover
- mock_controller.go: extract associateTaskPRRequest struct,
buildTaskPRFromRequest, and ensureMockPRForRequest helpers so the
handler is back under the 80-line Go limit.
- pr-ci-popover.tsx: remove unreachable PRAggregatePopover export.
Multi-PR tasks currently use the existing DropdownMenu trigger
(no hover popover), so the aggregate component was dead code.
* fix(github): drop unused anchorRef + tests for is{NotFound,Forbidden}Err
- pr-topbar-button.tsx: anchorRef was leftover from the dropped
click-to-pin onInteractOutside guard. Removed.
- gh_client_test.go: tests for the new isNotFoundErr / isForbiddenErr
helpers covering each accepted format and a few non-matches so a
future gh CLI wording change is caught.
* fix(github): UnresolvedReviewThreadsPopulated guard + drop unreachable nil checks
- service.go: mirror the ChecksPopulated/preserve dance for
UnresolvedReviewThreads. The REST single-PR poller (getPRStatus)
doesn't fetch review threads, so without this guard every REST
sync would clobber the GraphQL-populated value to 0 and emit a
spurious github.task_pr.updated event. The GraphQL path
(convertBatchedPRResult) sets the new flag.
- branch_protection.go: drop unreachable s == nil / fetcher == nil
checks. fetchRequiredReviews is only invoked from SyncTaskPR on a
service constructed via NewService, which always initialises the
cache. After a successful type assertion, the interface value
cannot be nil; the !ok branch already covers nil-interface inputs.
- service_test.go: new TestSyncTaskPR_UnresolvedThreads_PreservesOnLightweightSync
covers the regression. Existing NewFieldsPublishEvent threads-change
case updated to set UnresolvedReviewThreadsPopulated.
* refactor(web): drop CI popover progress bar — last-updated is enough
The animated progress bar was visual noise; the "updated Ns ago"
timestamp tells the user the same thing without the indicator. Footer
also hides entirely until the first fetch lands so an empty popover
doesn't show a useless "" line.
* fix(web): background-sync PRFeedback so popover is fresh on first hover
The on-hover SWR fetch was the only refresh path, so opening the popover
showed the previously-cached value while a new fetch raced behind it —
visibly stale until the refetch landed. The button icon, by contrast,
updates the moment a github.task_pr.updated WS push lands.
Add a usePRFeedbackBackgroundSync hook mounted at the PRTopbarButton
that subscribes to the same pr.updated_at change signal and refetches
PRFeedback into the cache in the background. The popover hook still
fires an on-open refetch as a safety net for the very first hover
(before any sync). No extra polling — fetches are gated on real
TaskPR changes, matching the icon's cadence.
* feat(web): CI status chip in chat status bar + tweak elapsed label
- New <PRStatusChip> renders a compact colored circle in the chat
status bar (left of the TodoIndicator, just above the input). Hover
opens the same PRCIPopover anchored to the chip's top edge so the
card expands upward — matching the user's "open bottom-up" ask. The
chip mounts the background-sync hook so the popover is fresh on
first hover even when the top-bar PR button isn't visible.
- elapsedLabel: "just now" → "updated just now" so the footer reads
the same regardless of how recently the cache was refreshed.
* fix(web): chip color was missing — use static class map for Tailwind JIT
\`getPRStatusColor().replace("text-", "bg-")\` produced class strings at
runtime, which Tailwind's content scanner doesn't pick up — the chip
rendered with no background. Replace with a static lookup so each
literal `bg-*` class appears in the source and gets compiled into the
bundle.
* fix(web): chip uses currentColor SVG so the dot actually renders
The previous bg-* approach relied on Tailwind compiling the literal
class strings. Even with a static map some palette entries (notably
bg-muted-foreground) didn't surface as a usable utility, so the chip
remained transparent. Apply getPRStatusColor's text-* class to the
button wrapper and fill an SVG circle with currentColor — the text-*
utilities are already compiled (PRTaskIcon uses them) so the dot
is guaranteed to render in the right color.
* fix(web): redesign PR status chip — clipboard icon + status glyph
- Move chip to right of TodoIndicator (was left).
- Render as "[clipboard icon] [status glyph]" instead of a bare circle:
passed → green CircleCheck, failed → red CircleX, in_progress →
yellow Loader2 (spinning), merged/neutral → muted dot. Status icons
reuse already-compiled text-* utilities so colors render reliably.
* fix(github): ReviewCount = approved-reviewer count + ReviewCountsPopulated guard
Two bugs feeding the popover's "Approved (0)" misreport:
- Semantic: REST path set ReviewCount to len(reviews), which double-counts
authors who left multiple reviews and includes COMMENTED rows. Replace
with countApprovedReviewers — distinct authors whose latest review
state is APPROVED. GraphQL converter gets the parallel
countApprovedReviewerNodes helper and also populates PendingReviewCount
from ReviewRequests.TotalCount (was unset, so the chip's "(K requested)"
suffix was always missing).
- Clobber: GraphQL polls used to write ReviewCount/PendingReviewCount=0
on every cycle because the converter never set them. Add
ReviewCountsPopulated to PRStatus and gate SyncTaskPR's writes on it
the same way ChecksPopulated already does, so a partial sync path
preserves the persisted values instead of resetting them to zero.
* fix(web): popover nitpicks (icon, review row, indent, badge)
- Chip icon: ClipboardCheck → ListCheck.
- PRReviewRow always renders (drop the early-return). Fresh PRs with
no approvals + no protection rule now show "Awaiting review" instead
of vanishing.
- Indent workflow rows under the In Progress / Failed group headers
(pl-5) so the hierarchy reads more clearly.
- In-progress workflow badge: "0/1 ran" → "1 running". The old phrasing
hid whether the job had even started.
* feat(web): stacked pass-rate progress bar in CI popover
Mirrors TodoIndicator's progress bar at the top of the CI section, but
split into three coloured segments:
green (passed) | yellow (in_progress) | red (failed)
sized proportionally to each bucket's count. The label reads
"Pass rate · K/N (P%)" so the user gets the same at-a-glance summary
TODOs shows. Skipped/cancelled-but-not-failed jobs aren't counted —
the bar always reflects the actionable picture.
* fix(web): right-align review counts + drop chip hover bg
- Move approval/required counts to the right edge of the review row
("Approved" on the left, "1 / 2" on the right), mirroring the
bucket-row layout. The "(K requested)" suffix becomes a "· K requested"
trailing fragment on the right side.
- PRStatusChip: drop hover:bg-accent/50 so the trigger doesn't grow a
background plate when the popover is open.
* fix(web): switch CI chip icon to IconChecklist
* fix(github): clear bot review-comment backlog
Real correctness fixes:
- pr-status-chip: pending checks / pending review now beat
checks_state === "success" in chipStatus precedence — a PR with
CI green but reviewers outstanding is no longer rendered as
"passed".
- branch_protection: drop the unreachable Reset() method.
- pr-topbar-button: stop double-mounting usePRFeedbackBackgroundSync;
the chip already covers it.
- use-pr-ci-popover: dedupe key combines prFeedbackKey + updated_at
so switching tasks with same-timestamp PRs still refetches.
- use-pr-ci-popover: drop stale comment about the removed progress bar.
Refactor:
- client_helpers + graphql: extract reviewSample +
latestReviewStateByAuthor helper so the three near-identical
per-reviewer dedup loops share one implementation.
Tests:
- countApprovedReviewers + reduceReviewSummary unit tests.
- TestSyncTaskPR_ReviewCounts_PreservesOnLightweightSync +
AcceptsRealZero mirror the existing ChecksPopulated guards.
- groupChecksByWorkflow distinct-URL test verifies the failed-job
URL-selection fix.
- branch_protection TTL tests now use testing/synctest — instant
virtual time, no real-sleep flake on CI.
* fix(web): clicking the CI chip no longer closes the popover
HoverCard treats the trigger button as outside the content's bounding
box, so a click on the chip fired Radix's onPointerDownOutside and
auto-closed the popover. Add a triggerRef + onPointerDownOutside guard
on HoverCardContent that filters out clicks landing on the trigger —
clicking the chip is now a no-op while the popover stays open via hover.
|