Commit Graph

8 Commits

Author SHA1 Message Date
Bohan Jiang
6dba74c3c1 fix(task): auto-retry transient "Connection closed mid-response" like chat (MUL-4910) (#5565)
* fix(task): auto-retry transient provider stream cut like chat (MUL-4910)

Claude Code's "API Error: Connection closed mid-response" is a transient
network cut. In unattended issue runs it fell through to
agent_error.unknown / process_failure — neither in retryableReasons — so
the task terminated. Interactive chat only appeared resilient because the
CLI's own in-process retry usually recovers first; there is no
chat-specific retry in Multica. Both paths share the same
finalizeStreamResult -> Classify -> retryableReasons pipeline.

- classify: route "connection closed" / "mid-response" to
  agent_error.provider_network, before the exit-status rule so the
  "exited with error: exit status N" variant also lands here.
- retry: add provider_network to retryableReasons. It is resume-safe, so
  the retry child inherits the session and continues the truncated
  conversation instead of restarting.

Tests cover the new classification (incl. exit-status variant) and the
retry/resume flags. Note: mirror the substrings into the MUL-1949 offline
backfill SQL to keep in-flight and historical taxonomies aligned.

Co-authored-by: multica-agent <github@multica.ai>

* feat(task): defer provider_network's final retry ~5s — three-tier (MUL-4910)

Follow-up to the immediate-retry fix: make the connection-closed retry a
three-tier schedule — first run + immediate retry + one retry deferred ~5s —
so a blip that survives the immediate retry gets a short cooldown before the
final attempt instead of firing back-to-back.

Reuses the existing deferred/fire_at primitive (the comment-routing escalation
mechanism) rather than adding new infrastructure: CreateRetryTask gains an
optional fire_at; when set, the child is inserted 'deferred' and the existing
PromoteDueDeferredTasksForRuntime sweeper — already run promote-first on every
claim poll — flips it to 'queued' at fire_at. No migration, no claim-query
change, no daemon change.

- retryAttemptCeiling: raise provider_network's ceiling to 3 (other reasons
  keep max_attempts=2); applied in both retryEligible and
  MaybeRetryFailedTask's budget pre-check so the primary and sweeper paths agree.
- retryDelayForAttempt: only provider_network's final attempt is deferred (5s);
  every other retry — including provider_network's first — stays immediate.
- FailTask + MaybeRetryFailedTask pass fire_at and skip the queued
  broadcast/notify for a deferred child (promotion emits them at fire time).

Timing: the deferred child fires on the first claim poll at/after fire_at, so
>= 5s; on an otherwise-idle runtime it can stretch to the poll interval — the
same behaviour deferred escalations already have.

Tests: pure schedule/eligibility coverage (TestProviderNetworkRetrySchedule)
plus a DB test asserting fire_at controls deferred-vs-queued, attempt=3, and
resume-safety.

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): persist reason-aware retry budget; respect max_attempts=1 disable (MUL-4910)

Addresses the pre-merge review must-fix: retryAttemptCeiling raised
provider_network to 3 unconditionally, which (1) overrode the
max_attempts=1 "auto-retry disabled" contract and (2) persisted a
self-contradictory child (attempt=3, max_attempts=2) that leaks to the
task API, so a naive attempt < max_attempts consumer would misjudge the
budget.

- retryAttemptCeiling now returns taskMaxAttempts unchanged when it is <= 1
  (disabled stays disabled) and only ever WIDENS otherwise (max(col, 3) for
  provider_network) — a higher configured budget is kept.
- CreateRetryTask takes an optional max_attempts; FailTask and
  MaybeRetryFailedTask write the reason-aware effective ceiling into the
  child so the whole retry chain self-describes (attempt=3, max_attempts=3).
  NULL inherits the parent column, so non-provider_network reasons are
  unchanged.

Tests:
- pure: ceiling widens to 3, keeps a higher budget, and never revives a
  disabled (max_attempts=1) task; eligibility rejects the disabled case.
- DB (end-to-end FailTask): default budget → deferred final child at
  attempt=3/max_attempts=3; first failure → immediate child; max_attempts=1
  → no child. Plus CreateRetryTask persists the passed budget / inherits on NULL.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-17 16:47:59 +08:00
Bohan Jiang
ae68799c64 feat(task): reuse workdir on manual retry, gate session on error class (MUL-4869) (#5525)
* feat(task): reuse workdir on manual retry, gate session on error class (MUL-4869)

A manual retry (execution-log "retry" button -> POST /api/issues/{id}/rerun)
previously set force_fresh_session=true unconditionally, so the daemon threw
away the failed run's workdir and started from an empty one. For a transient
failure (network blip, provider 5xx / rate-limit, runtime_offline, timeout)
that discards work the agent already did.

New contract: a retry ALWAYS reuses the source task's workdir when it still
exists on disk; only the agent SESSION is gated, on whether the source task's
failure poisoned the conversation.

- RerunIssue derives force_fresh_session from the source task's failure_reason
  via resumeUnsafeFailureReason instead of hardcoding true. Transient/cancelled
  failures resume the session; conversation-poisoning failures start fresh.
- The daemon claim handler resolves the retry's session/workdir precisely from
  rerun_of_task_id, not the most-recent (agent,issue) row that a parallel task
  could hijack. PriorWorkDir is returned whenever present; PriorSessionID only
  when resume-safe AND on the same runtime.
- force_fresh_session now gates only the session, never the workdir.
- Add agent_error.context_overflow to the resume-unsafe set (Go helper plus the
  GetLastTaskSession / GetLastChatTaskSession blacklists): resuming an overflowed
  context would immediately overflow again.

Objectively-unreusable cases (workdir GC'd, different runtime, failed before a
workdir was recorded) fall back to a fresh Prepare via the daemon's existing
execenv.Reuse / gateResumeToReusedWorkdir path, so no daemon change is needed.

Tests: RerunIssue force_fresh classification by failure_reason; claim-layer
workdir-always-reused plus session gating (same/different runtime, poisoned);
context_overflow classifier.

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): make manual-retry workdir reuse rollback-safe and error-text aware (MUL-4869)

Addresses code review on #5525.

- Rollback safety: RerunIssue no longer writes force_fresh_session based on the
  source failure. Rerun rows are pinned to force_fresh_session=true again, so an
  OLD claim handler picked up mid rolling-deploy degrades to a clean start
  instead of falling back to the (agent, issue) most-recent lookup and resuming
  a *different* execution. The new claim handler ignores the flag for reruns and
  computes session reuse from the exact source task (rerun_of_task_id).

- Legacy poison defense: add shared service.ResumeUnsafeFailure(failureReason,
  errorText), which combines the failure_reason poison set with the same
  400/invalid_request_error raw-error-text guard GetLastTaskSession applies. The
  claim handler's exact-source path uses it, so legacy 'agent_error' /
  deploy-window rows carrying a 400 marker are no longer resumed.

- CI: TestRerunIssueTargetsSourceTaskAgent passes again (rerun rows stay
  force_fresh_session=true). Service test now asserts that rollback-safe
  invariant across failure classes; the claim test drives session gating from
  the source task and adds a legacy-400 case.

- Docs: tasks.{mdx,zh,ja,ko}.mdx and the GetLastTaskSession SQL comment now
  distinguish execution-log per-row retry (task_id: reuse workdir, conditional
  session) from CLI/API rerun (no task_id: fresh session + fresh workdir).

- Clarify cross-runtime workdir is best-effort: the daemon offers the source
  workdir regardless of runtime (a shared mount may resolve it) and only the
  per-cwd session is runtime-gated.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-16 16:38:18 +08:00
Multica Eve
cf1fc64671 MUL-4824: fail closed on incomplete Claude-style result streams (#5522)
* fix(agent): fail closed on incomplete stream results

Co-authored-by: multica-agent <github@multica.ai>

* fix(agent): tighten empty result fallback

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-16 15:22:46 +08:00
iYuan
2f1f90c11a fix(agent): retry codex semantic inactivity fresh (#2593) 2026-05-20 20:03:39 +08:00
LinYushen
b7a58c06ac Revert "feat(task): wire claim lease into TaskService and sweeper (MUL-2246) …" (#2673)
This reverts commit bb32be0e50.
2026-05-15 16:06:58 +08:00
LinYushen
bb32be0e50 feat(task): wire claim lease into TaskService and sweeper (MUL-2246) (#2662)
* feat(task): wire claim lease queries into TaskService and sweeper (MUL-2246)

- ClaimTask now uses ClaimAgentTaskWithLease (generates claim_token + lease)
- StartTask accepts optional claim_token for token-verified start
- AgentTaskResponse includes claim_token for daemon to use
- Daemon client sends claim_token in StartTask body
- Sweeper calls RequeueExpiredClaimLeases each tick
- Legacy daemons without claim_token still work (graceful fallback)

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): address PR #2662 review blockers (MUL-2246)

1. ClaimAgentTaskForRuntime: push runtime_id into atomic SQL WHERE clause
   so runtime A cannot claim tasks queued for runtime B under the same agent.

2. Legacy StartAgentTask: add claim_token IS NULL guard so leased rows
   cannot be started without token verification. Handler rejects malformed
   tokens with 400 instead of silently degrading to legacy path.

3. StartAgentTaskWithClaimToken: validate claim_expires_at >= now(),
   preserve claim_token until terminal state (only clear claim_expires_at),
   use CTE + UNION ALL for idempotent retry when daemon resends after a
   lost StartTask response. Return 409 Conflict on token mismatch/expiry.

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): StartTask 409 handling, transport retry, claim_token on FailTask (MUL-2246)

- StartTask 409 (claim superseded): release slot, don't call FailTask
- StartTask transport timeout/5xx: retry once with same token, then
  check task status before failing
- FailTask now sends claim_token; server-side FailAgentTask SQL adds
  AND (claim_token IS NULL OR claim_token = @claim_token) guard so
  stale daemons cannot fail tasks that have been re-claimed

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): close FailTask token bypass and RequeueExpiredClaimLeases liveness gap (MUL-2246)

Blocker 1 - FailTask token validation:
- SQL: change (param IS NULL OR claim_token = param) to
  (param IS NULL AND claim_token IS NULL) OR claim_token = param
  so tokenless requests can only fail legacy (tokenless) rows.
- task.go: malformed claim_token now returns ErrInvalidClaimToken (400)
  instead of being silently dropped to NULL.
- Handler: maps ErrInvalidClaimToken→400, ErrClaimTokenInvalid→409.
- Service: when UPDATE returns no rows but task is still active,
  return ErrClaimTokenInvalid (token mismatch) instead of silent success.

Blocker 2 - RequeueExpiredClaimLeases runtime liveness:
- SQL: JOIN agent_runtime, only requeue tasks where runtime is 'online'.
  Dead/offline runtime tasks stay dispatched for FailTasksForOfflineRuntimes.
- FOR UPDATE → FOR UPDATE OF atq (required with JOIN).

Regression tests:
- task_claim_token_test.go: malformed, tokenless-on-tokened, wrong-token
- requeue_lease_test.go: SQL must JOIN agent_runtime with online filter

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): move expired lease requeue to ClaimTaskForRuntime preflight, add heartbeat freshness backstop (MUL-2246)

- Add RequeueExpiredClaimLeasesForRuntime: per-runtime preflight self-requeue
  in ClaimTaskForRuntime. Runtime proves liveness by actively claiming, so no
  heartbeat check needed.
- Update global RequeueExpiredClaimLeases to require ar.last_seen_at freshness
  (stale_threshold_secs param). Prevents requeuing to a dead runtime in the
  90s gap between lease expiry (60s) and offline detection (150s).
- Add regression tests verifying the heartbeat freshness check and that the
  preflight query does not join agent_runtime.

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): use LivenessStore for global requeue, move preflight before empty-cache (MUL-2246)

Blocker 1: Global RequeueExpiredClaimLeases now uses LivenessStore.IsAliveBatch
to verify runtimes are truly alive before requeuing expired leases. When
LivenessStore is unavailable (no Redis), global requeue is skipped entirely —
the preflight self-requeue in ClaimTaskForRuntime handles live runtimes. This
closes the 60-150s gap where a dead runtime still appears online in DB.

Blocker 2: Moved RequeueExpiredClaimLeasesForRuntime BEFORE EmptyClaim.IsEmpty
fast-path in ClaimTaskForRuntime. Expired leases are now requeued (which bumps
the empty cache via notifyTaskAvailable) before the empty check can
short-circuit the claim path.

Also adds ListRuntimesWithExpiredClaimLeases SQL query and LivenessChecker
interface on TaskService.

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): wire EmptyClaimCache into backend taskSvc for backstop requeue (MUL-2246)

The backend taskSvc used by the sweeper only had Liveness wired but not
EmptyClaim. When global backstop requeue called notifyTaskAvailable,
s.EmptyClaim.Bump() was a nil no-op — the handler's empty-cache was never
invalidated, so the daemon's next claim hit a stale empty verdict.

Fix: wire the same Redis-backed EmptyClaimCache into the backend taskSvc
in main.go (same Redis keys as router.go:139 handler instance).

Add regression test verifying backstop requeue invalidates the handler's
empty-cache.

Co-authored-by: multica-agent <github@multica.ai>

* fix(task): global backstop must not requeue — alive runtimes use preflight, dead stay dispatched (MUL-2246)

- RequeueExpiredClaimLeases is now a no-op (returns 0 always)
- Alive runtimes self-requeue via ClaimTaskForRuntime preflight
- Dead runtimes stay dispatched for FailTasksForOfflineRuntimes
- Rewriting to queued on dead runtime creates 2h blackhole (offline
  sweeper only handles dispatched/running)
- Test actually calls RequeueExpiredClaimLeases and asserts 0 in all cases

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): remove duplicate usage reporting block after merge conflict (MUL-2246)

The merge resolution introduced a second ReportTaskUsage call after the
status check, duplicating the usage-before-early-return block that already
runs right after runner.run. Remove the duplicate and add a regression test
asserting /usage is called exactly once on the normal completion path.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: multica-agent <github@multica.ai>
2026-05-15 15:15:31 +08:00
LinYushen
0b1333fb00 feat(server): orphan-task recovery + auto-retry + manual rerun (MUL-1128) (#1476)
* feat(server): orphan-task recovery + auto-retry + manual rerun (MUL-1128)

When the daemon process crashed mid-task the issue was stuck at
in_progress for up to 2.5h: the in-flight task timeout was the only
mechanism that ever moved the row, and the runtime heartbeat sweeper
only fires after the runtime stays offline for 45s — a quick restart
beats both windows.

This change implements the A+B plan from the issue thread:

A. lifecycle hygiene
- migration 055 adds attempt / max_attempts / parent_task_id /
  failure_reason / last_heartbeat_at to agent_task_queue
- new daemon-auth endpoint POST /runtimes/{id}/recover-orphans:
  daemon calls it on every register so the server fails any
  dispatched/running tasks the previous process left behind
- new daemon-auth endpoint POST /tasks/{id}/session: persists the
  agent's session_id + work_dir mid-flight so a crash doesn't
  lose the resume pointer (claude+codex emit MessageStatus with
  SessionID; daemon forwards on the first one it sees)
- FailAgentTask / FailStaleTasks / FailTasksForOfflineRuntimes
  now set failure_reason ('agent_error' / 'timeout' /
  'runtime_offline')

B. auto-retry with resume context
- TaskService.MaybeRetryFailedTask spawns a fresh queued attempt
  carrying parent's session_id/work_dir when the failure reason
  is infrastructure-shaped (timeout, runtime_offline,
  runtime_recovery) and attempt < max_attempts; skips autopilot
- wired into the runtime sweeper paths and TaskService.FailTask
  so the user transparently sees a new in_progress run instead of
  a stuck row
- new user-auth POST /api/issues/{id}/rerun + multica issue rerun
  CLI for the manual escape hatch

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(server): address PR review for orphan-task recovery (MUL-1128)

Three review-must-fix items on top of the A+B implementation:

1. recover-orphans now funnels through TaskService.HandleFailedTasks,
   the same shared post-failure pipeline used by the runtime sweeper.
   This guarantees task:failed events are emitted, agent status is
   reconciled, and issues stuck in_progress with no remaining active
   task are reset to todo even when no auto-retry is created
   (max_attempts exhausted, autopilot, non-retryable reason).

2. RerunIssue now uses CancelAgentTasksByIssueAndAgent, scoped to the
   issue's current assignee. The previous implementation called
   CancelAgentTasksByIssue, which would collateral-cancel parallel
   @-mention agents on the same issue.

3. GetLastTaskSession now considers both completed and failed tasks
   (mirroring GetLastChatTaskSession), ordering by the most recent
   timestamp. With UpdateAgentTaskSession pinning session_id/work_dir
   mid-flight, an auto-retry or manual rerun of a daemon-crash failure
   now actually resumes the prior conversation context instead of
   starting fresh — matching the stated B-branch behaviour.

go build / go vet pass; the existing service and agent test suites pass.
runtime_sweeper / handler integration tests require a local DB with the
055 migration (and the pre-existing 050 first_executed_at column).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-22 13:08:37 +08:00
Kagura
1a565a221a fix(server): handle race in CompleteTask and FailTask for parallel agents 2026-04-21 19:23:31 +08:00