Delete unreachable/unused backend symbols confirmed by staticcheck U1000
and golang.org/x/tools/deadcode, plus deprecated back-compat aliases kept
alive only by their own tests.
Removed (unused):
- parseGitHubRepoURL, syncTaskStateForBulkWorkflowMove (task/service)
- resolvePrimaryRepoInfo (orchestrator/executor)
- resolveWorkflowAgentProfile, workflowDefaultAgentProfile wrappers (mcp/handlers)
- cleanupExitedContainer (runtime/lifecycle) + orphaned fmt import
- VscodeManager.setEnv (agentctl/server/process)
- repoRootDir const (cmd/preview)
Removed (deprecated shims):
- Canceller.CancelSessionAndNotify alias -> callers use DetachSessionAndNotify
- ScriptPlaceholder type alias (live scriptPlaceholders var kept)
- Placeholder{QueueRun,ClearDecisions,QueueRunForEachParticipant}Callback
no-op structs and their self-referential test
No behavior change; build, vet, and affected-package tests pass.
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(executor): re-clone provider repos when local path is missing or not a git repo
Previously the re-clone guard only fired when local_path was empty, so a
set-but-invalid path (e.g. deleted checkout, or relocated by the provider
clone-path/identity change) was fed straight to the worktree preparer,
which hard-failed with "repository is not a git repository" instead of
recovering.
Two-part fix:
- resolveTaskRepoInfo now also re-clones when local_path exists but isn't
a valid git checkout (new isLocalGitRepo helper), self-healing on next
launch.
- validateWorktreeRequest now fails the "Validate repository" step with an
honest error naming the offending path when it truly isn't a git repo,
instead of passing and cascading into a misleading "sync interrupted"
failure downstream.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(executor): gate stale-path reclone to provider repos and cover resume + multi-repo
Restrict the stale-local-path re-clone to genuinely provider-backed repos
(SourceType != "local") so a local checkout that's temporarily missing is
never silently overwritten. Centralize the re-clone guard into a shared
ensureRepoLocalPath helper and call it from the single-repo resume path
(applyResumeRepoConfig) so resume self-heals like a fresh launch. Apply the
same non-git-path validation used by the single-repo prepare path to the
multi-repo prepareOneRepo path so a stale secondary repo fails fast with a
clear message instead of surfacing worktree.Manager's ErrRepoNotGit.
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>
* feat(plugins): add capability-gated Host reader for conversation content
Plugins can now read historical user/agent message content over the Host
data API, gated by a new `api_read:messages` capability (ADR 0047). This
unblocks "summarize yesterday"-style plugins that the live `message.added`
bus event alone cannot serve.
- proto: `ListMessages` RPC + `Message`/`MessageFilter` on `service Host`.
- SDK: `Host.Messages() MessageReader`, Go-native `Message`/`MessageFilter`.
- host: `api_read:messages` accessor + denied reader; content sanitized via
`sysprompt.StripSystemContent` (no raw system prompts); RFC3339 `since`/
`until` → `InvalidArgument` on bad input.
- data: new `ListMessagesForPlugin` filtered query (session/task ids, type,
created_at range) through the task service, never the DB file.
- tests: in-process + wire capability gating, content stripping, filter/time
passthrough, pagination, and the SQLite query.
- docs: ADR 0047, spec, plugins-manifest, plugins-authoring.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(plugins): add Host.InvokeUtilityAgent for settings-selectable utility agent
Plugins can now delegate a one-shot LLM step to an operator-configured agent,
gated by a new `agent_invoke` capability (ADR 0048), so a plugin needs no
provider API key of its own.
- setting: new `utility_agent_profile_id` user setting (Settings > System >
Utility Agent), a profile dropdown cloned from the existing utility-agent
path; selectable in the UI.
- capability: boolean `agent_invoke`, enforced like `state`/`secrets`.
- proto/SDK: `InvokeUtilityAgent(prompt) -> text` RPC and
`Host.InvokeUtilityAgent(ctx, prompt)`.
- backend: reuses the sessionless host-utility inference tier (ADR 0002) via a
thin adapter — no agent loop, no task/session/workspace; resolves the
configured profile to (agent type, model). Unconfigured / deleted profile ->
gRPC FailedPrecondition.
- tests: capability gating, completion path (asserts the configured profile is
used and text returned), no-utility-agent and stale-profile error paths,
SDK/wire round-trips, and the settings apply/trim behavior.
- docs: ADR 0048, spec, plugins-manifest, plugins-authoring.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(plugins): resolve utility-agent deps live and bound message filter size
Address CI + review feedback on the host plugin capabilities:
- docs: register the new /settings/system/utility-agent route in
docs/public/coverage.json (fixes the "Validate public docs" CI failure).
- utility agent (ADR 0048): hostUtilityMgr is wired late in boot — after
StartActivePlugins has already spawned boot-active plugins — so snapshotting
the utility deps in hostForPlugin stranded those hosts with nil deps and made
InvokeUtilityAgent return Unimplemented for their lifetime. Read the deps live
from the Service (mutex-guarded) at call time instead; add a regression test
for late wiring reaching an already-spawned host.
- messages (ADR 0047): cap the combined session/task/type filter values at 400
so an oversized filter fails fast with InvalidArgument instead of hitting
SQLite's host-parameter limit at query execution; add a test.
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>
* 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.
* 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>
* chore: remove orphan executor tracing file and deprecated ResolveScript wrappers
- Delete internal/agentctl/tracing/executor.go (7 TraceExecutor* funcs)
- Delete internal/orchestrator/executor/script_template.go (deprecated wrapper)
- Delete scriptengine.ResolveScript convenience function
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove orphan setter methods across healthpoll, jira, linear, slack, updates
- Delete Poller.SetInterval from integrations/healthpoll (no callers)
- Delete Poller.SetIssueTickHook from jira and linear (no callers)
- Delete Trigger.SetInterval and Trigger.SetMatchHook from slack (no callers)
- Delete Service.SetPollInterval from system/updates (no callers)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove orphan wrapper functions and accessor method
- Delete updates.FetchLatestRelease shim (wraps FetchLatestReleaseFrom, no callers)
- Delete usage.NewClaudeUsageClient and NewCodexUsageClient shims (no callers)
- Delete database.Service.Path() accessor (no callers)
- Update ErrGitHubRateLimited godoc to reference FetchLatestReleaseFrom
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove unwired handleListExecutors MCP handler
The sibling profile handlers (list/create/update/delete) are registered
via RegisterFunc in handlers.go. handleListExecutors never was — dead flow.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove stale hook-field comments after setter deletion
SetIssueTickHook (jira, linear) and SetMatchHook (slack) were removed
in the deadcode cleanup. Their struct fields still claimed "tests use
this" — no test or caller can set these fields now.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove orphan hook fields and fire methods after setter deletion
issueTickHook (jira/linear) and matchHook (slack) are unexported and
never set after their public setters were removed; fireIssueTickHook /
fireHook were permanent no-ops.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(task/repo): split sqlite/base.go (1486 lines) into schema/migrations/queries
base.go grew to 1.85x the 800-line lint threshold by mixing the Repository
struct, schema initialization, ALTER TABLE migrations, recreate-and-copy
table rebuilds, and historical backfills/heals. Split along those seams:
- base.go (58 lines) keeps only the Repository struct, NewWithDB,
newRepository, Close, DB.
- base_schema.go owns the initSchema orchestrator, every init*Schema
function, ensure*Indexes, ensureRunnerProjectionTables, and the
plan-revision backfill that runs as part of initPlansSchema.
- base_migrations.go owns migrate*, recreateTable[Named], runMigrations,
the backfillRow type, and every backfill*/heal* function. Verbatim
function moves with no semantic changes.
- base_queries.go owns sqlLimitClause (still referenced by plan.go,
document.go, message.go).
To keep golangci-lint (new: true) green after the move:
- initSchema's 17 if-return chain becomes a for-each-step loop so
cyclomatic complexity drops from 19 to within the 15 limit.
- initInfraSchema and initSessionWorktreeSchema hoist their DDL strings
into package-level consts (infraSchemaDDL, sessionWorktreeSchemaDDL)
so each function body fits the 80-line funlen cap.
Step ordering in initSchema is preserved exactly (backfillTaskEnvironments
-> backfillTaskEnvironmentRepos -> healTaskEnvironmentWorkspacePaths ->
healDuplicateTaskEnvironments -> ensureTaskEnvironmentTaskUniqueIndex ->
healSessionTaskEnvironmentIDs) so existing prod DBs continue to migrate
correctly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat(task/repo): add BatchGetSessionsByTaskIDs to eliminate task-list N+1
Adds a single repository method that returns every task_session for a list
of task IDs in one chunked IN (...) query, grouped by task_id. The
workspace task-list endpoint previously needed three batched calls
(GetPrimarySessionIDsByTaskIDs + GetSessionCountsByTaskIDs +
GetPrimarySessionInfoByTaskIDs); with this loader the handler can derive
primary session ID, session count, and per-task session info from a
single round trip.
- buildInPlaceholders and chunkIDs land in base_queries.go.
sqliteMaxHostParams=500 keeps each chunk well below SQLite's
compile-time SQLITE_MAX_VARIABLE_NUMBER (999 on older builds, 32766
on newer).
- session.go gains BatchGetSessionsByTaskIDs, which reuses
taskSessionSelectCols / scanTaskSessions / loadWorktreesBatch so the
returned sessions are indistinguishable from ListTaskSessions output
(worktrees populated, JSON snapshots decoded). Empty input
short-circuits with a non-nil empty map.
- SessionRepository interface and Service.BatchGetSessionsForTasks
expose the new method to the handler layer. The three existing
*ByTaskIDs methods are left in place because service_events.go still
uses them for single-task event publishing.
- mockRepository implementations updated in two test files so the
packages still satisfy the interface.
TestBatchGetSessionsByTaskIDs covers the empty-input short-circuit, the
multi-task (3 x 2) case (primary derivation + no cross-task bleed), and
the asked-for-but-missing case (absent from result map).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* perf(task/handlers): use BatchGetSessionsForTasks in task-list endpoint
buildTaskDTOsWithSessionInfo previously issued three batched queries per
request (GetPrimarySessionIDsForTasks + GetSessionCountsForTasks +
GetPrimarySessionInfoForTasks). Each was already chunked by task_id,
but together they were three round trips for data that can be derived
from one. Collapse to a single BatchGetSessionsForTasks call and derive
primary session ID, count, and per-task primary info in memory.
The JSON response shape is unchanged: dto.FromTaskWithSessionInfo
still receives the same pointer-typed arguments, primarySessionID stays
nil when no primary exists, and sessionCount stays nil for tasks with
zero sessions (matches the existing pointer-when-present contract that
the frontend store hydration depends on).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task/handlers): restore JOIN-derived executor type/name in task-list response
buildTaskDTOsWithSessionInfo regressed when it stopped calling
GetPrimarySessionInfoForTasks: extractSessionInfo reads
ExecutorSnapshot["executor_type"] and ExecutorSnapshot["executor_name"],
but those keys exist only on the synthetic snapshot built by the
GetPrimarySessionInfoByTaskIDs LEFT JOIN to executors. The persisted
ExecutorSnapshot JSON uses different keys (e.g. "type" — see
share/builder.go:96), so BatchGetSessionsForTasks alone always yielded
empty executor_type / executor_name fields in the task-list response,
breaking the frontend task card's executor badge.
Fix (caught in code review):
- Handler now calls both BatchGetSessionsForTasks (for primary session
ID and session count) and GetPrimarySessionInfoForTasks (for the
JOIN-derived executor type/name and other session info). Two queries
instead of the original three; correctness preserved.
- New TestGetPrimarySessionInfoByTaskIDs_PopulatesExecutorJoinFields
seeds an executor + primary session, then asserts the JOIN populates
ExecutorSnapshot["executor_type"] / ["executor_name"] on the returned
session. Locks in the contract the handler depends on so a future
refactor that drops the JOIN fails loudly instead of silently
emitting empty fields.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task/repo): chunkIDs returns empty slice for empty input
chunkIDs previously returned [][]string{nil} for an empty/nil input
(a single chunk whose value is nil). The only current caller
(BatchGetSessionsByTaskIDs) short-circuits empty input before reaching
chunkIDs, so the bug stayed dormant — but any new caller that forgot
the short-circuit would feed a nil chunk to buildInPlaceholders (which
returns "" for nil), produce a `... WHERE task_id IN () ORDER BY ...`
query, and hit a SQL syntax error at runtime.
Make the helper safe by construction: an empty input returns nil
(empty outer slice), so `range chunkIDs(empty, n)` iterates zero
times. Add base_queries_test.go covering:
- Empty/nil input → no chunks (locks the safety contract).
- Single-pass input fits in one chunk.
- Over-threshold input splits into correctly-sized, in-order chunks.
- buildInPlaceholders normal + empty-input cases.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task/repo): propagate non-ErrNoRows errors in backfillSingleTask executor lookup
`backfillSingleTask` looked up `executors.type` for the legacy executor
referenced by a session and defaulted to `"local_pc"` on any error.
That silently masked driver failures, schema mismatches, and corruption
— every backfilled task_environment would get the wrong executor_type
without surfacing the underlying problem.
Narrow the default to the legitimate case: only `sql.ErrNoRows` (the
executor row was actually deleted, e.g. legacy session whose executor
is gone) falls back to `"local_pc"`. Any other scan error now aborts
the migration with a wrapped error so operators see the real cause.
Adds TestBackfillSingleTask_DefaultsExecutorTypeOnMissingRow which
seeds a session referencing a non-existent executor id and asserts the
backfill still completes with `executor_type = local_pc` — locks in
the legitimate-fallback path.
Addresses PR #1058 review feedback from cubic & claude.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task/repo): chunk ListWorktreesBySessionIDs to honor SQLite var limit
ListWorktreesBySessionIDs built its IN-clause from every input session ID
in one shot. BatchGetSessionsByTaskIDs chunks task IDs at 500 to stay
below SQLITE_MAX_VARIABLE_NUMBER (999 on older builds), but its
loadWorktreesBatch step then handed all sessions in that 500-task chunk
to ListWorktreesBySessionIDs un-chunked — so at modest workloads (500
tasks × ≥2 sessions each = 1000 placeholders) the query fails with
SQLITE_RANGE and bubbles up as a 404 on the task-list endpoint.
ListWorktreesBySessionIDs now applies the same chunkIDs / 500 cap that
BatchGet uses. Body extracted to appendWorktreesForSessionChunk so the
public method stays under the 80-line funlen cap.
New TestListWorktreesBySessionIDs_ChunksOverPlaceholderLimit feeds the
method 1001 non-existent session IDs and asserts it returns cleanly —
without chunking, this fails with SQLITE_RANGE on a default-built SQLite.
Also splits TestBatchGetSessionsByTaskIDs (was 98 lines) into focused
subtests + a shared seedBatchSessions helper, satisfying the test-size
convention. The three sub-scenarios (empty input, multi-task grouping,
missing-task absence) now fail in isolation.
Addresses PR #1058 review feedback from greptile (P1) and claude
(suggestion).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task/repo): port repositories.copy_files migration to split files
Rebased onto main; main added copy_files to runMigrations() and
initTaskSchema() in base.go while our branch had split those into
base_migrations.go and base_schema.go. Cherry-pick both additions
into the correct split files so the schema is consistent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* 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>
Carlos Florêncio