* fix(task): cascade-archive child tasks on workflow delete
Tasks pointing at a deleted workflow_id silently became unreachable
because the FK on tasks.workflow_id was dropped to support ephemeral
quick-chat tasks, so SQLite cannot CASCADE. Service.DeleteWorkflow
now archives every active child task before removing the workflow,
re-using the existing ArchiveTask path (events, worktree cleanup,
agent stop). Frontend confirm copy and button label updated to reflect
the new behavior.
Closes#1279
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(task): make workflow-delete cascade race-safe
Address review feedback (Greptile, Claude, cubic): ArchiveTask now returns
a typed ErrTaskAlreadyArchived sentinel so DeleteWorkflow can swallow the
TOCTOU race where a task is archived between ListTasks and ArchiveTask.
Without the sentinel, that race would propagate as an opaque error and
leave the workflow row intact even though every task in the snapshot was
already in the desired state.
Tests:
- TestService_DeleteWorkflow_SkipsConcurrentlyArchivedTask covers the
sentinel-skip path through the cascade.
- TestService_DeleteWorkflow_PartialArchiveErrorPreservesWorkflow covers
the fail-fast contract on a real (non-sentinel) error: workflow row
survives, previously-archived tasks stay archived, retry is safe.
- TestService_ArchiveTask_ReturnsAlreadyArchivedSentinel locks in the
sentinel-error contract DeleteWorkflow relies on.
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>
* fix: gate step_complete_kandev tool on per-step signal flag
Two ADR-0015 follow-up bugs:
- "Wait for agent completion signal" checkbox snapped back because
updateWorkflowStepAction dropped auto_advance_requires_signal from the
PUT body. Add it to the Pick<>, body builder, BackendWorkflowStep, and
transformWorkflowStep.
- step_complete_kandev MCP tool was injected into every first-turn
prompt, polluting context and tempting agents on non-signal-gated
steps to call it. Replace the static description in
kandev-context.md with a {step_complete_section} placeholder filled
only when the current step has auto_advance_requires_signal=true.
Add orchestrator.Service.StepRequiresCompletionSignal helper and
thread it through all four InjectKandevContext call sites.
* fix: address PR 1300 review feedback
- Hoist ProcessOnTurnStart above the first-turn MCP wrap so an
on_turn_start step transition is reflected in the
auto_advance_requires_signal flag baked into the stored/forwarded
prompt (coderabbit, cubic).
- Add Service.WorkflowStepRequiresCompletionSignal step-ID variant and
use it from task_operations.go / autoStartStepPrompt to skip the
redundant GetTask round-trip at hot first-turn launch sites (claude).
- Add table-driven tests covering every silent-false branch and the
positive path of both helpers (greptile, claude, cubic).
* docs(sysprompt): document stepCompleteSection trailing newline contract
Address Claude review suggestion on PR 1300: make explicit that the
constant must end with "\n" because the kandev-context.md template inlines
{step_complete_section} immediately before the next bullet, so the
trailing newline is what separates the two list items.
* fix: restore task-id lookup at startTask wrap site
Address PR 1300 review regression: the previous swap to
WorkflowStepRequiresCompletionSignal(ctx, workflowStepID) silently hid
step_complete_kandev on manual starts where the workflowStepID parameter
is empty but the task is already bound to a signal-gated step in the DB.
The `task` variable in scope at this call site is `*v1.Task` (from the
scheduler) and does not carry WorkflowStepID, so the cheaper step-ID
variant is not applicable here — go back through StepRequiresCompletionSignal
so the repo lookup pulls the canonical step.
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
* feat(workflow): explicit step-completion signal for auto-advance
Adds `step_complete_kandev` MCP tool + `WorkflowStep.auto_advance_requires_signal`
column so workflow steps can gate their `on_turn_complete` transitions on an
agent-emitted completion signal instead of inferring "done" from any turn-end.
See docs/decisions/0015-explicit-completion-signal-for-auto-advance.md for the
motivation, alternatives considered, and trade-offs.
* fix(workflow): address PR review feedback for explicit completion signal
- Dedup `LoadPendingStepSignal` into `internal/task/models` so the MCP
handler (write site) and the orchestrator (read site) share one decoder.
- Gate the out-of-band subscriber on `WorkflowStep.AutoAdvanceRequiresSignal`
so `step_complete_kandev` is not an unintended manual-advance path for
signalless steps.
- Fail closed on step-load errors in `processOnTurnCompleteViaEngine`: a
loader hiccup must block the transition rather than silently bypass the
gate.
- Reject `step_complete_kandev` for terminal sessions
(Completed / Failed / Cancelled) so the agent gets a clear error instead
of `accepted: true` followed by a no-op.
- Surface bus-publish failures back to the agent instead of swallowing
them — the idempotency guard makes the retry safe and prevents the
session from getting stuck.
- New `TestOnStepCompletionSignaled` covers the subscriber paths
(RUNNING / WAITING+match / stale step / not signal-gated).
- ADR doc fixes: payload field list now matches the wire shape, writer
ownership is documented, the unsubstantiated 250ms-debounce claim is
removed, and the late-signal re-close race is recorded as a deferred
open question.
- Drop the new toggle's HelpTip in favour of an inline description — the
shadcn tooltip portal was intercepting clicks on the agent-profile
dropdown options in the workflow settings E2E.
* fix(workflow): round-2 review fixes for explicit completion signal
- Split `handleStepComplete` into `resolveStepCompleteTarget` +
`publishStepCompletionEvent` helpers so the entrypoint clears funlen
(88 → under 80 lines) and the two halves are independently testable.
- Classify session/task load errors instead of returning `NotFound` for
every failure: use the exported `taskrepo.ErrTaskNotFound` sentinel for
task lookups, substring-match for the session repo (no sentinel today),
and surface anything else as `InternalError` with the raw error logged
but not leaked to the agent — so a transient DB hiccup triggers a retry
rather than session abandonment.
- Pin `step_complete_kandev` explicitly in the mode-specific tool-count
tests: `Contains` in `ModeTask`, `NotContains` in `ModeConfig` and
`ModeOffice`, so accidental dereg or cross-mode registration is caught
by the diff instead of by a still-equal count.
* fix(workflow): round-3 review fixes + defer pipeline-editor toggle
- Defer the "Wait for agent completion signal" sub-toggle to a follow-up
PR. Adding it to the step config card shifts the layout enough that the
agent profile select's Radix popper opens into a sibling tooltip's hover
path, breaking workflow-agent-profile.spec.ts on shard 7. The backend
column ships in this PR — workflows can opt in via the YAML import or
the MCP `update_workflow_step_kandev` tool — and the UI affordance gets
its own iteration where the test interference can be investigated
without blocking the rest of the change.
- Re-publish the bus event on the `already_signaled` short-circuit when
the session is WAITING_FOR_INPUT. Without this, an agent's retry after
the first call's publish failed would short-circuit on the persisted
bag, never fire the subscriber, and leave the session stuck.
- handleStepComplete docstring: drop the non-existent feature-flag claim
and the "audit entry" wording — gating is purely
`WorkflowStep.AutoAdvanceRequiresSignal` and the bag is cleared on the
next turn start with no separate audit trail.
- `step_complete_kandev` tool description: same fix — clarify that the
call succeeds for non-gated steps but the signal is discarded on the
next turn start (no audit history to query later).
* test(workflow): cover handleStepComplete + reword defer-comment
- Add `TestHandleStepComplete_*` (6 subtests, all passing) covering the
uncovered handler paths flagged by Claude/cubic on round 3:
missing-fields validation, session-does-not-belong-to-task ownership
guard, terminal-session rejection (Completed/Failed/Cancelled),
first-call happy path (bag persisted + bus payload shape pinned), dedup
while RUNNING (no re-publish), and dedup while WAITING_FOR_INPUT (the
retry-after-publish-failure path — must re-publish so the subscriber
fires).
- Reword the deferred-toggle comment to drop the YAML-export claim:
the workflow export model doesn't yet serialize
`auto_advance_requires_signal`, so MCP `update_workflow_step_kandev`
is the only configuration path that round-trips today. Note the
follow-up needs to extend the export schema alongside the UI toggle.
* test(workflow): pin signaled_at + drop redundant GetTaskSession on bag-clear
- Assert `signaled_at` is present and parses as RFC3339Nano in
`TestHandleStepComplete_FirstCallAccepted`. The field is part of the
handler's documented response contract; pinning it catches accidental
drop/rename regressions.
- Add `clearPendingStepSignalByID(ctx, sessionID)` as a thin DB-only
variant of `clearPendingStepSignal` and use it from
`executeStepTransition`, where the session struct isn't read after the
clear. Avoids an extra `GetTaskSession` round-trip per transition.
`clearPendingStepSignal(session)` still exists for the
in-memory-mutation case (subscriber's stale-step path).
* fix(workflow): subscriber stale-clear guard + ADR/comment drift fixes
- onStepCompletionSignaled's stale-step path now reads the bag and only
clears it when its current StepID matches the (stale) event's stepID.
Previously the unconditional clear could destroy a valid signal that
the new step's agent had just written when sessions are reused across
steps without auto_start_agent. (Greptile P1.)
- Drop the "feature flag" phrase from WorkflowStep.AutoAdvanceRequiresSignal
docstring — leftover from an earlier draft. The gating is purely the
boolean field; the global flag was removed two rounds ago.
- Merge the two adjacent `if triggerOnEnter` blocks in
executeStepTransition so the full triggerOnEnter=true path reads top-
to-bottom in one place. (CodeRabbit + cubic.)
- ADR 0015 Mechanism section: re-label the SessionStepHistory.Metadata
audit annotation, the `step_complete_signaled` system message, and the
expvar telemetry counters as deferred. They're planned shapes, not
current behaviour — calling them "Audit" / "Chat visibility" /
"Telemetry" in present tense was misleading for future readers.
* fix(workflow): drop subscriber complexity + reload session on stale-clear
- Extract `parseStepCompletionEvent` and `stepIsSignalGated` helpers from
`onStepCompletionSignaled`. Cyclomatic complexity goes 17 → under 15
(cyclop on new code, CI lint regression after the round-6
stale-clear guard).
- The stale-step branch now re-loads the session from the DB before
deciding whether to clear the bag. The previous round-6 fix checked
the local `session.Metadata` snapshot, which can be older than a
concurrent write from the new step's agent — that check could see a
stale signal that had since been overwritten with a valid one and
erase the new bag entry. The narrower load-on-clear is cheap and
closes the cubic P1 hole. (The broader race in
`processOnTurnCompleteViaEngine` reading session.Metadata is the
monotonic-marker open question in ADR 0015; out of scope here.)
* test(workflow): pin stale-event guard's preserve-current-bag side + dedup StringFromAny
- Add `TestOnStepCompletionSignaled` subtest "stale event → valid bag
for CURRENT step is preserved" — fires a stale step-A event while
the bag holds a valid step-B signal, asserts the bag still contains
StepID=step_b after the subscriber runs. Pins the negative side of
the round-6 `existing.StepID == stepID` guard, so a regression that
drops or inverts the guard (and would silently leave signal-gated
steps stuck) fails the test instead of slipping through.
- Promote `pendingSignalString` to exported `models.StringFromAny`
and reuse it from the orchestrator subscriber's
`parseStepCompletionEvent`. Eliminates the byte-for-byte duplicate
helper across the two packages; the bag-shape decoder and the bus-
event decoder now share one source of truth for "missing string".
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
* 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.
Carlos Florêncio