wsStatus, wsListInstances and wsTest discarded the ParsePayload error and
carried on with a zero-valued SecretID, so a malformed frame surfaced as a
confusing downstream failure (or a lookup against an empty secret ID) instead
of a clear bad-request. Match the sibling handlers in the same file and return
ws.NewError(..., ErrorCodeBadRequest, "invalid payload", nil).
Absent payloads keep working: Message.ParsePayload treats a nil payload as a
no-op, so only genuinely malformed JSON now errors.
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four templates were confirmed against the shipped CLI, not inferred
from the agent name:
- pi-acp -> {home}/.pi (pi-acp@0.0.32 package source)
- qoder-acp -> {home}/.qoder (qodercli 1.1.7, --config-dir proof)
- kiro-acp -> {home}/.kiro (Kiro CLI 2.15.0, ACP session/new)
- trae-acp -> {home}/.cache/trae-cli (Trae CLI 0.120.48, ACP session/new)
Kiro and Trae deliberately break the "{home}/.<agent>" shape: Kiro keeps
its login token in ~/.local/share/kiro-cli/data.sqlite3 while sessions
live in ~/.kiro, and Trae keeps config in ~/.trae but session state in
the cache dir with auth in the OS keyring. Each Runtime() records the
evidence and the residual gaps.
Extends the shared ACP agent spec table with sessionDirTemplate so every
agent's session root is pinned, including the ones already set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GetBlocking unconditionally returned an empty slice behind a TODO, so any
caller asking "which tasks does this task block?" silently got a wrong
answer. The reverse-lookup query the TODO asked for already exists on the
blocker repository as ListTasksBlockedBy (used by the office scheduler and
the handoff context builder) and is already part of the BlockerRepository
interface, so GetBlocking now delegates to it.
Like GetBlockers, it reports the raw blocker edges without filtering
archived tasks — every other ListTasksBlockedBy consumer does the same.
ListTasksBlockedBy also gains a deterministic ORDER BY (mirroring
ListTaskBlockers) and normalizes an empty result to a non-nil slice.
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
`apps/web/lib/state/store.ts.backup` was a 1202-line snapshot of the
pre-slice-refactor monolithic Zustand store, accidentally committed in
b090a0157 (#1049). It never compiled and never ran — `tsconfig.json`
includes `**/*.ts`, which does not match a `.backup` suffix — so its only
effect was polluting grep/search results for anyone working on the real
store at `apps/web/lib/state/store.ts`.
No runtime code referenced it: `grep -rn "store.ts.backup" apps/` returns
nothing, and no build, lint, or tsconfig glob picks up `*.backup`.
Verified no logic was dropped. Of the 177 state keys and actions in the
backup, 172 exist in the live slice tree under `lib/state/slices/`. The
five that don't belong to two concepts that were deliberately renamed
after the snapshot was taken:
- `boardId` / `setBoards` / `setActiveBoard` — boards became workflows in
658bd349b (#228); the live kanban slice uses `workflowId` /
`setWorkflows` / `setActiveWorkflow`.
- `environmentsLoaded` / `setEnvironments` — environments became
executors; the live settings slice has `ExecutorsState` /
`setExecutors`.
Also add `*.backup`, `*.bak`, and `*.orig` to `.gitignore` so editor and
merge scratch copies cannot be committed again.
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
http.Get uses http.DefaultClient, which has no timeout and carries no
context. A backend that accepted the TCP connection but never responded
blocked waitForHealth forever, hanging the launcher with no way to
cancel it -- exactly what the suppressed noctx lint was warning about.
Probe through a package-level client with an explicit 2s timeout using
http.NewRequestWithContext, thread a context down from launcher.Run, and
drain each response body before closing so the connection is reused
across poll iterations. The overall wait now derives a deadline from the
caller's context, so cancellation ends it too.
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(backend): bound tool installer downloads with timeouts
The GitHub release and tarball install strategies downloaded archives with
http.DefaultClient, which has no timeout: a slow or stalled mirror left an
install hanging indefinitely with no way out.
Both strategies now share a package-level client whose transport bounds each
phase that can stall silently (connect, TLS handshake, response headers), and
whose response bodies are wrapped in a stall guard that aborts the transfer
when it stops making progress. There is deliberately no overall
http.Client.Timeout — these are tarballs, and one deadline for the whole
transfer would kill a large-but-healthy download on a slow link.
Requests keep using http.NewRequestWithContext, so an install remains
cancellable; the guard derives its own cancellable context from the caller's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(backend): bound only blocked reads in the download stall guard
The guard armed its timer at construction and rearmed it after any read that
returned bytes, so the window also covered whatever the caller did with those
bytes downstream. Expanding one read's worth of a highly compressed archive
onto a slow disk can outlast the window without the mirror having gone quiet
at all, which would surface a false "download stalled" error on a perfectly
healthy install.
Arm the timer around the blocking read alone and stop it as soon as the read
returns, so the bound is on how long a read blocks rather than on wall-clock
time spent downstream.
Reported by Codex review on #2016.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(web): streamline mobile review file headers
* fix(web): toggle mobile review file menu
* test(web): select mobile review header in grouping test
* fix(terminal): expose executor-profile env vars to terminal shells
PR #1971 got the profile env into the agent subprocess and the repository
setup script, but the terminal panel was still a gap: the local user shell
was started with `InteractiveStartRequest.Env` unset, so it inherited only
the backend process environment, and agentctl's per-terminal shells were
started from `shell.DefaultConfig` with no env overrides at all.
Local/worktree path: resolve the task environment's executor profile at
terminal start (revealing secret-backed entries) and pass it through
UserShellOptions.Env into the deferred PTY start. Resolution happens per
terminal rather than being cached at launch, so a terminal opened before
the agent ever runs still gets the vars.
Remote path (Docker/Sprites/SSH): StartTerminalShell now folds the agent
instance's env into the shell config, so a terminal inside the workspace
sees what the agent sees. SSH is unchanged in scope — its instance env is
still only the credential allowlist, not arbitrary profile variables.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(terminal): resolve terminal env from the session's executor profile
Address PR review. The resolver read executor_profile_id off the
task_environments row, but persistTaskEnvironment's reuse branch never
refreshes that column — it keeps whatever the first session stamped on it.
buildLaunchAgentRequest resolves the agent's env from
session.ExecutorProfileID instead, so a task environment reused by a later
session that picked a different profile of the same executor type would give
the terminal stale secrets and omit the ones the agent just received.
Prefer the session's profile and fall back to the environment row only when
the session recorded none. Also document why mergeAgentEnvIntoShellConfig
needs no defensive copy and why StartEnvForTesting is exported.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(terminal): give env-row fallback subtests distinct session IDs
Address PR review nit: both subtests shared the hardcoded "session-3" at
the call site, so the error-path case read as a repeat of the first.
Move the session ID into the table as session-no-profile / session-error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(mcp): honor explicit task_id in plan/walkthrough/review tools
resolveTaskID silently discarded the caller-supplied task_id whenever the
MCP server was session-bound (every agent session) and substituted the
caller's own task. A parent reading a child's plan got the parent's empty
plan back ("No plan exists"), and a cross-task create/update would have
written to the wrong task — all with no error, despite the tool schemas
advertising task_id as a usable parameter.
Honor an explicitly provided task_id and fall back to the session-bound
task only when the argument is absent, matching how message_task_kandev /
stop_task_kandev already address other tasks. Cross-task access stays safe:
internal/mcp/scope attaches the stream owner's identity (from the agent's
own execution, never the payload), and the plan/walkthrough/review services
gate the target task_id through the same workspace-ownership check every
other cross-task surface uses — an out-of-reach task is rejected, not
served. The review service gains the SetTaskAuthorizer wiring the plan and
walkthrough services already had, so publish_review_findings is covered too.
Tool descriptions now document the default-to-current / cross-task-within-
reach behavior. Tests cover the resolution precedence (explicit wins /
fallback to bound / neither errors), a session-bound-to-A reads/creates a
plan for B regression, and the review authorizer allow/deny paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(mcp): drop mcp.Required() from task_id on session-defaulting tools
The plan, walkthrough, and publish-review tools default task_id to the
session-bound task when it is omitted, so advertising it as required
contradicted that documented behavior (flagged by Codex and CodeRabbit on
the PR). Mark task_id optional on all eight; handler behavior is unchanged.
The genuinely cross-task tools (update/move/delete/archive_task,
message_task, get_task_conversation) keep task_id required.
Adds a schema test asserting task_id is present-but-optional on every
session-defaulting tool.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(mcp): cover review scoping + walkthrough/review tool-desc reach
Addresses two non-blocking review findings:
- Extend TestPlanAndWalkthroughScoping to exercise ReviewService through the
assembled AuthorizeTaskAccess → SetTaskAuthorizer → PublishFindings path, so
all three task-scoped services are covered uniformly (foreign owner denied
with ErrTaskNotFound; real owner passes and the finding is stored).
- Document the default-to-current / cross-task-within-reach semantics in the
top-level descriptions of the walkthrough and publish-review tools (was only
in the task_id parameter description), since LLMs read the tool description
first. Broaden the description test to assert all eight session-defaulting
tools carry it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(mcp): assert review finding persists through the auth gate
Follow-up to review: query repo.ListTaskReviewFindings after the owner
publish in TestPlanAndWalkthroughScoping and assert exactly one finding was
stored, so the test verifies the write landed in the repository rather than
only the service's returned slice.
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): implement Host data API write RPCs (CreateTask/UpdateTask/CreateComment)
The api_write manifest capability was a stored no-op. Wire the deferred
phase-2 write RPCs (ADR 0043) so closed-loop plugins (Sentry->task,
Slack slash-command task creation, refactor fan-out) can create and
update tasks and post comments.
- SDK: Host.Tasks() gains Create/Update; new Host.Comments() writer;
CreateTaskInput/UpdateTaskInput/Comment DTOs (pkg/pluginsdk).
- Gating: api_write:tasks gates Create/UpdateTask, api_write:comments
gates CreateComment; undeclared -> gRPC PermissionDenied, per-method
and independent of the read caps on the same resource.
- Writes route through the first-party service layer (task service for
tasks, office service for comments) via backendapp adapters, never a
repository -- so task.*/comment-created events fire and WS updates.
Server stamps source="plugin:<id>"; CreateTask resolves workspace/
workflow defaults and best-effort honors start_agent; UpdateTask uses
a conservative title/description/state/workflow_step_id mask.
- Fixture plugins exercise a CreateTask+CreateComment round-trip;
in-process, real-broker wire, and subprocess tests cover the round-trip
and per-capability denial.
- Docs: spec, GRPC-CONTRACT, HOST-DATA-API.proto, proto comments moved
writes deferred -> implemented; ADR 0043 amended with the concrete
service-layer routing decision.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(plugins): send a message to a task session via Host SendMessage (replaces CreateComment)
The office TaskComment write was the wrong primitive — it's an office/
dashboard construct, not a way to message the agent working a task, and
it only did anything under office reactivity. Replace the CreateComment
write RPC with SendMessage, which delivers a prompt to a task session
through the orchestrator's real delivery path (the same one message_task
uses).
- Proto: swap CreateComment/Comment for SendMessage(SendMessageRequest)
returns SendMessageResponse{session_id, status}; regenerated stubs.
- SDK: Host.Comments() removed; Host.Messages().Send(taskID, sessionID,
text) added (pairs with Messages().List), returning MessageDispatch.
- Gating: api_write:comments dropped; SendMessage gated api_write:messages
(independent of api_read:messages on the same accessor, like Tasks()).
- Delivery: a new pluginsTaskMessengerAdapter (backendapp) resolves the
target session (explicit id verified against the task, or the primary
session), records a user message stamped source="plugin:<id>", then
dispatches by state — running → queue; idle/completed → prompt (resume
if the agent process is gone); never-started → launch; failed dispatch
deletes the recorded message. Wired via SetWriteDeps (live-read).
- Tests: adapter state-machine unit tests (queue/start/prompt/resume/
delete-on-failure/belongs-to-task/no-primary/terminal); SDK, in-process
gating, wire, and subprocess round-trips updated to SendMessage.
- Docs: spec, GRPC-CONTRACT, HOST-DATA-API.proto, proto comments, and the
ADR 0043 amendment updated from comment-create to message-send.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(plugins): address PR review — nil guards, detached contexts, state validation
Follow-up fixes from the #1970 review (Claude + CodeRabbit bots):
- grpcHostServer.CreateTask/UpdateTask/SendMessage: nil-guard the returned
value before toProto() so a custom/test Host returning (nil, nil) yields a
gRPC Internal error, not a server panic (matches GetTask's pattern).
- host.go: fix stale server doc comment (Comments()/CreateComment →
Messages()/SendMessage).
- startTaskBestEffort and the post-resume wait+retry now run on
context.WithoutCancel(ctx): the task/resume is already committed, so a
plugin's incoming deadline must not silently abort the launch/resume.
- pluginsTaskWriterAdapter.UpdateTask validates the state string against the
known task-state enum (excluding orchestrator-owned SCHEDULING) and returns
InvalidArgument, so a plugin typo can't forward a bogus state to the service.
- Tests: add the resume-wait-timeout path (message deleted so a retry can't
duplicate), and a dedicated pluginsTaskWriterAdapter test covering
source→metadata mapping and state validation (both adapters now depend on
narrow interfaces for testability).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(plugins): launch start_agent asynchronously so CreateTask doesn't block
Review nit follow-up: startTaskBestEffort called StartTask synchronously, so
with the detached context the plugin's CreateTask RPC blocked on the
orchestrator's full inline launch path (office lookup, state update, workflow
step move, task fetch) with no bound — and the "asynchronous" comment was wrong.
Move the fire-and-forget into pluginsTaskStarterAdapter (backendapp, no goleak),
mirroring MCP's launchAutoStartTask: the launch runs in a goroutine on a
context.WithTimeout(context.WithoutCancel(ctx), AgentLaunchTimeout), and
StartTask returns immediately. The plugin host stays a simple synchronous call
(its taskStarter contract now documents "returns promptly, launches async").
Adds adapters_plugin_starter_test.go: launch invoked, returns before the launch
completes, detaches from a cancelled caller context, and swallows launch errors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(plugins): document write validation + SendMessage rollback; harden starter test cleanup
Addresses the final PR #1970 review nits:
- adapters_plugin_starter_test.go: register the block-channel release via
t.Cleanup (sync.Once-guarded) before the goroutine can block, per AGENTS.md.
- GRPC-CONTRACT.md: document CreateTask/UpdateTask validation + error rules
(title required, id required, NotFound on missing task, invalid-state reject).
- spec.md: document that a failed SendMessage dispatch deletes the recorded
message, plus the required-field/NotFound/terminal-session rules.
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(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>
* fix(worktree): export executor-profile env into repo setup script
Executor-profile environment variables (with secrets revealed) were merged
into the launch request env and reached the agent subprocess, but the
repository setup script — run inside worktree.Manager.Create, where
`npm install` typically runs — was built with only the install-managed env
(GOCACHE). The profile env was never injected, so tokens like an npm auth
token were unavailable during install. This affected both single-repo and
multi-repo tasks, since both run their setup script through Create.
Thread the resolved profile env into the setup script at the point both
paths converge: add CreateRequest.ScriptEnv (transient, never persisted so
secrets stay out of the DB), set it from req.Env in buildWorktreeCreateRequest
(used by single-repo and per-repo in multi-repo), and merge it over the
managed env in runWorktreeSetupScript. Managed GOCACHE keeps precedence.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(worktree): avoid overflow-flagged alloc size, clarify env precedence
Address PR review: drop the len(a)+len(b) capacity hint in mergeScriptEnv
(CodeQL flagged the addition as a possible allocation-size overflow) and
reword the ScriptEnv doc comment to state that install-managed values such
as GOCACHE take precedence over profile values on key collisions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(worktree): round out mergeScriptEnv edge cases
Add profile-only and managed-only assertions to TestMergeScriptEnv per PR
review, making the table exhaustive alongside the existing (nil,nil) and
(profile+managed) cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Carlos Florêncio