feat(tasks): continue a finished background run in chat - #6248
Conversation
Watching a background agent run is currently read-only: the execution log
and the agent activity tab show status and a transcript, and that is the end
of the interaction. To ask the agent one follow-up question you either
comment on the issue and wait for a whole new run to cold-start, or open a
chat with that agent that knows nothing about the work you just read.
The pieces to fix this already existed. chat_session has carried session_id,
work_dir and runtime_id since migrations 033/060, and the daemon's chat claim
resolves its resume pointer straight off those three columns. So continuing a
run in chat needs no new execution machinery — only a chat_session seeded
with the finished task's pointer.
Adds POST /api/tasks/{taskId}/continue-in-chat plus a button on both surfaces
that list agent tasks.
Deliberate constraints:
- Terminal tasks only. A live run still owns its provider session and its
work_dir, resuming an ACP session from a second client is undefined, and a
reused work_dir has no mutual exclusion (only local_directory tasks take a
path lock). Non-terminal tasks get 409 task_not_terminal rather than a
silent degrade.
- runtime_id comes from the source task, not the agent. Claim only resolves a
chat resume when chat_session.runtime_id equals the claiming task's
runtime, so copying the agent's current binding would silently discard the
session whenever the agent has since been re-bound.
- The session pointer is withheld when the source failure cannot survive a
resume (service.ResumeUnsafeFailure) or its rollout never landed, and the
response reports session_carried=false so the UI says so. Several backends
only report their session at completion; a chat that claims continuity it
does not have is worse than no button.
- Idempotent per (task, member) via a partial unique index, so a second click
reopens the conversation instead of forking a second one onto the same
provider session.
The column is a soft reference and the index is CONCURRENTLY in its own
migration file, per developers/conventions.zh.mdx.
Co-authored-by: multica-agent <github@multica.ai>
|
@vokako is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
…er case CI's frontend-build failed on the previous commit: the button test built its status fixtures from bare string arrays, so `status` widened to `string` and would not assign to AgentTask["status"]. One of those arrays also carried 'deferred', which the DB CHECK permits but the TS union does not. The array is now a total Record over AgentTask["status"], so a status added to that union fails to compile here until it is classified rather than silently defaulting to "not continuable". The wider DB domain, including 'deferred', stays enumerated in TestIsTerminalTaskStatus where the authoritative list lives. Root cause of it reaching CI: `tsc` ran before this test file existed and was never re-run, and vitest strips types rather than checking them, so a green test run said nothing about type correctness. Verified this time with the exact CI command, `turbo build typecheck lint`. Also from internal review: - Document why the unique index is (task, creator) and not (task) alone, so the per-creator scoping is not mistaken for an oversight and "fixed" into handing member B member A's private conversation. Adds TestContinueTaskInChat_SecondMemberGetsOwnConversation to pin it. - Note in the button why a 403 is read via `reason_code` while the 409s carry a plain `reason`. Co-authored-by: multica-agent <github@multica.ai>
The handler tests call ContinueTaskInChat directly and inject the workspace and member context by hand, so they prove nothing about the layers in front of it: a route that was never registered, or registered outside the authenticated group, passes all of them. That is the gap that let a broken build reach CI once already, so close it with a test that exercises the real router. Goes through httptest.NewServer(NewRouter(...)) with a real bearer token and asserts an unauthenticated call gets 401, an authenticated one gets 201, the persisted chat_session actually carries the task's session_id / work_dir / runtime_id rather than merely echoing them, and a second call reopens. Verified the test has teeth: with the route line removed it fails with 404 on both the unauthenticated and authenticated assertions. The first version of this test borrowed the shared fixture's agent via `WHERE workspace_id = ... LIMIT 1`. Sibling tests in this package create and archive agents in the same workspace, so it was order-dependent — an archived agent turned the expected 201 into a 400, which passed in isolation and failed in the full package run. It now seeds its own runtime + agent + invocation target. Confirmed stable over five consecutive full-package runs. Co-authored-by: multica-agent <github@multica.ai>
|
Pushed two follow-up commits. Summary of what changed and why, since the first run of this PR was red.
My fault, and worth stating plainly: I ran Router-level coverage (added in That test also caught a bug in itself worth mentioning, since it is the kind of thing that rots a suite: the first version borrowed the shared fixture's agent via From internal review, two clarifications rather than behaviour changes: the All 10 checks are green on |
What does this PR do?
Watching a background agent run is currently read-only. The Execution log on an issue and the agent Activity tab show status and a transcript, and that is where the interaction ends. To ask the agent one follow-up question you either comment on the issue and wait for a whole new run to cold-start, or open a chat with that agent that knows nothing about the work you just read.
This adds a Continue in chat action on finished runs. The new conversation inherits that task's provider session, working directory and runtime, so the first message resumes the run instead of starting from nothing.
Thinking path. The interesting part of this change is how little of it is new.
chat_sessionhas carriedsession_id,work_dirandruntime_idsince migrations 033 and 060, and the daemon's chat-claim branch resolves a resume pointer straight off those three columns (server/internal/handler/daemon.go, thetask.ChatSessionID.Validbranch). Issue tasks record exactly the same three values. So "continue this run in chat" does not need a new execution path, a new daemon signal, or any change to how runs are claimed — it needs achat_sessionrow seeded with a finished task's pointer, and a button. Everything else here is the constraints that make that safe, which are the parts worth reviewing.Related Issue
No upstream issue — issue creation is restricted in this repository, so there was nowhere to file it first. Happy to open one if a maintainer would prefer the discussion to live there.
Type of Change
Changes Made
Server
server/migrations/251_chat_session_continued_from_task.{up,down}.sql— addschat_session.continued_from_task_id.server/migrations/252_chat_session_continued_from_task_index.{up,down}.sql— partial unique index on(continued_from_task_id, creator_id).server/pkg/db/queries/chat.sql—CreateChatSessionContinuingTask(the existingCreateChatSessionderivesruntime_idfrom the agent and cannot accept a resume pointer) andGetChatSessionContinuingTaskfor the idempotency lookup. Regenerated with sqlc v1.31.1.server/internal/handler/task_continue_chat.go— newContinueTaskInChathandler.server/internal/handler/chat.go—ChatSessionResponse.continued_from_task_id.server/cmd/server/router.go—POST /api/tasks/{taskId}/continue-in-chat, next to the existing task cancel route.Frontend
packages/views/common/continue-in-chat-button.tsx— sharedContinueInChatButton+canContinueTaskInChat, placed incommon/alongsideTranscriptButtonso both task-listing surfaces use one implementation.packages/views/issues/components/execution-log-section.tsx— button onPastRow.packages/views/agents/components/tabs/activity-tab.tsx— button in the activity row's hover actions.packages/core/api/client.ts,packages/core/types/chat.ts,packages/core/types/index.ts—continueTaskInChat()andContinueTaskInChatResult.packages/views/locales/{en,zh-Hans,ja,ko}/common.json—continue_in_chat.*. Incommonrather thanissuesbecause the component is shared across two feature namespaces.Docs
apps/docs/content/docs/tasks{,.zh,.ja,.ko}.mdx— one bullet in "Viewing run history", including the honest caveats.Design decisions worth reviewing
Terminal tasks only; non-terminal returns 409. A live run still owns its provider session and its working directory. Resuming an ACP session from a second client is undefined for the long-lived backends, and a reused
work_dirhas no mutual exclusion — onlylocal_directorytasks take a path lock (acquireLocalDirectoryLockIfNeeded);markActiveEnvRootis a GC refcount, not a mutex. So two runs could share one directory. The endpoint refuses withreason: task_not_terminaland the UI does not render the button for live rows.runtime_idcomes from the source task, not the agent. Claim resolves a chat resume only whenchat_session.runtime_idequals the claiming task's runtime. Copying the agent's current binding — which is whatCreateChatSessiondoes — would silently discard the session whenever the agent has since been re-bound. There is a dedicated test pinning this, because it is the kind of thing a later refactor "simplifies" back into a bug.The session pointer is withheld when it cannot work, and the response says so.
session_carried: falseis returned when the source failure cannot survive a resume (service.ResumeUnsafeFailure, the same judgment the rerun path applies) or when its rollout never landed (session_rollout_missing, MUL-5305). Also relevant: thekiro,kimi,qoderandtraeclibackends emit noMessageStatusat all, soPinTaskSessionnever fires for them and theirsession_idonly lands at completion — a task killed before its terminal report legitimately has awork_dirand no session. In every such case the chat still opens in the run's directory and the UI warns that the agent starts without the context. A chat that claims continuity it does not have is worse than no button, so the two halves of the pointer are tracked and reported independently.Idempotent per (task, member). Enforced by the unique index, not just the handler pre-check, because a double-click can lose that race. Two chats resuming one provider session is precisely the hazard this feature exists to avoid. A second call returns
200 { reopened: true }with the existing conversation.Permission uses the invoke gate, not the visibility gate. Continuing in chat starts agent runs, so it uses
canInvokeAgentlikeCreateChatSessiondoes — not the softer gateCancelTaskByUseruses. Being allowed to watch (or stop) a private agent's run must not imply being allowed to start new ones. Tenancy isGetAgentTaskInWorkspace, and cross-workspace ids 404 rather than 403 so the endpoint never confirms a task exists elsewhere.Conventions followed (
apps/docs/content/docs/developers/conventions.zh.mdx): no database foreign key —continued_from_task_idis a soft reference and readers tolerate a dangling id; the index isCREATE UNIQUE INDEX CONCURRENTLYalone in its own migration file, since the runner sends each file as one simple query and a multi-statement file would put it in an implicit transaction.Not in scope:
project_idis left NULL on the new session. It is a chat-specific context selection the member makes, and inheriting one would addLockProjectForChatSessionCreateplus a "project deleted meanwhile" failure path for no gain — what carries the run's context is the session and work_dir.How to Test
make migrate-upto apply 251 + 252.409 task_not_terminal.Local test results
go vet ./...— clean.gofmt -lon every changed Go file — empty.The 15 new Go tests, against a real Postgres:
Migrations were verified both directions on a scratch database, and the resulting schema inspected:
Frontend —
tsc --noEmitclean forpackages/coreandpackages/views;eslintclean on every changed/added file.The one failure is
layout/sidebar-resize.test.tsxand it is pre-existing and unrelated — it fails identically with this branch stashed, becauselocalStorageis unavailable in the Node build I ran it under (ExperimentalWarning: localStorage is not available because --localstorage-file was not provided). Everything else, includinglocales/parity.test.ts(160),execution-log-section.test.tsx(14),activity-tab.render.test.tsx(2) and the newcontinue-in-chat-button.test.tsx(6), passes.New tests
server/internal/handler/task_continue_chat_test.go— 6 DB-free tests over the decision logic (which half of the pointer is inherited, resume-unsafe withholding, the terminal-status domain enumerated from the CHECK constraint, rune-safe title truncation) and 9 DB-backed handler tests (pointer inheritance including the runtime-from-task assertion, idempotent reopen with a row count, all four non-terminal statuses refused with nothing created, resume-unsafe opening without a session, chat-task refusal, cross-workspace 404, private-agent 403 with no side effect, archived agent, title seeding).packages/views/common/continue-in-chat-button.test.tsx— 6 tests: the terminal-status gate across the whole status domain, navigation to the created session, the warn-but-still-navigate path when no session was carried, localized permission and non-terminal refusals, and re-enabling after failure.Checklist
apps/docs/content/docs/developers/conventions.zh.mdxRisks and gaps
MessagesSquare) added to the existing hover-action row on terminal task rows, next to the transcript and retry icons, on two surfaces — no layout change. I am happy to add screenshots if a maintainer wants them before review, and would rather say so than attach something misleading.(task, creator), so two members can each continue the same task and both chats resume the same provider session — sequentially fine, concurrently the same class of hazard as the live-task case, just much narrower. I chose per-creator scoping because chat sessions are private to their creator and a shared continuation would leak one member's conversation to another. Flagging it as a deliberate trade-off rather than an oversight.work_diris offered on purpose. If the directory was GC'd or is absent on the claiming runtime,execenvfalls back to a freshPrepare, so offering a path that may be gone is free. Noting it so it does not read as a missing check.AI Disclosure
AI tool used: Multica Agent (Kiro CLI backend), running as an agent inside a self-hosted Multica workspace.
Prompt / approach: The requesting user asked for "a button on a background agent run, or in the history, that jumps into a chat to continue" — after I had first misread the request as wanting mid-run message injection and produced a design for the wrong feature. He corrected the scope, I investigated whether the simpler thing was feasible, reported that the resume-pointer columns already existed and that the real constraints were the live-session/workdir hazards, and he approved implementing it.
I read the existing code before writing any:
CreateChatSessionas the transaction/permission template,CancelTaskByUserfor task tenancy, the daemon claim handler for how a chat resume is actually resolved, and each provider backend to establish which ones report a session mid-run. The constraints in this PR (terminal-only, runtime-from-task, withholding an unusable session, index-level idempotency) came out of that reading rather than from the original request, which asked only for a button. Two conventions I got wrong on the first pass and fixed after readingconventions.zh.mdx: I had added a foreign key, and I had put the index in the same migration file as theALTER TABLE.