Files
multica/server/cmd
Bohan Jiang 85a14cde37 fix(daemon): gate codex session pointer writes on rollout presence (MUL-5305) (#5960)
* fix(daemon): gate codex session pointer writes on rollout presence (MUL-5305)

Codex issue follow-ups on local_directory projects intermittently lost
their session: the server sent a prior session whose rollout was not in
the task CODEX_HOME, so the daemon dropped the resume and started a fresh
thread (gateCodexResumeToRolloutPresence), losing the conversation.

Root of the bad pointer: the daemon persists a Codex session id as the
resumable pointer at two points -- the mid-flight pin and the terminal
report -- before the rollout is guaranteed on disk. A task that exits
early (crash / runtime offline / timeout) leaves a pinned/reported
session id with no rollout; GetLastTaskSession (which accepts failed
rows) then hands it to the next follow-up, which drops it.

Enforce the invariant at write time: only record a Codex session as the
resumable pointer once its rollout is present in the per-issue store,
with a short bounded wait for flush. If it never lands, don't overwrite
the last good pointer -- a blanked session_id becomes NULL server-side,
so GetLastTaskSession falls back to the most recent session whose
rollout is real. Non-Codex providers are unaffected; crash recovery is
preserved because a present rollout still pins.

- codexSessionResumable: shared write-time presence check (bounded wait)
- runTask: gate the terminal session_id before reporting
- executeAndDrain: gate the mid-flight pin (thread codexHome through)
- tests: helper cases + behavioral pin test

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

* fix(daemon): address review — don't silently downgrade completed sessions (MUL-5305)

Follow-up to review feedback on #5960:

- Must-fix 1 (silent downgrade): limit the write-time session withholding
  to NON-completed terminal states. A missing rollout means no resumable
  conversation was persisted, so a withheld non-completed attempt loses
  nothing; a completed session is authoritative and, if its rollout is
  anomalously absent, is still recorded so the next run's resume gate
  discloses the loss (PriorSessionResumeUnavailable, MUL-4424) instead of
  silently falling back to an older session. Extracted
  resumableTerminalSessionID.
- Non-blocking risk: pin the mid-flight resume pointer with a per-status
  presence check instead of one fixed 2s window, and set sessionPinned
  only once the rollout is confirmed, so a rollout that lands shortly
  after the first status is still pinned this run.
- Must-fix 2 (regression coverage): pin skipped when rollout absent (no
  /session call); terminal helper (completed keeps / failed withholds);
  and a DB-backed GetLastTaskSession test proving the next claim falls
  back to the older recorded session when the latest was blanked.

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

* fix(daemon): disclose Codex session continuity gaps end-to-end (MUL-5305)

Addresses review feedback on #5960.

Must-fix 1 — a completed turn whose rollout is missing is exactly the
#5934 case (the reporter waits for each turn to finish), so it can no
longer be excluded from withholding. Withhold the session for ANY
terminal state, and pair the withhold with a persisted continuity-gap
signal so the next claim still discloses the loss even while resuming an
older good session:
  - new agent_task_queue.session_rollout_missing column (migration 224)
  - daemon sends session_rollout_missing on the terminal report; the
    handler clears the resume pointer (MarkTaskSessionRolloutMissing,
    overriding FailAgentTask's COALESCE) and flags the row
  - claim reads GetLatestTaskRolloutMissing and sets a new
    prior_session_resume_unavailable response field, which the daemon ORs
    into the brief's PriorSessionResumeUnavailable disclosure

Must-fix 2 — Codex reveals the session id on a single task_started
status, so a one-shot presence check missed a rollout that flushed later
and lost in-flight crash recovery. Pin via a background waiter bounded by
the run's context that pins the moment the rollout lands.

Tests: - completed + rollout missing -> next claim withholds the bad session
    AND flags the continuity gap (cross-layer DB test)
  - session pinned once its rollout appears after the status (mid-run)
  - pin skipped while the rollout is absent
Co-authored-by: multica-agent <github@multica.ai>

* fix(server): make continuity-gap write atomic + disclose on all claim paths (MUL-5305)

Addresses review round 3 of #5960.

Must-fix 1 — the previous handler-level marker ran AFTER the terminal
transaction committed, and FailTask creates + wakes the auto-retry inside
that same transaction, so a retry could claim the rollout-missing session
before the marker cleared it (and a marker failure was swallowed). Move
session_rollout_missing INTO the terminal write: CompleteAgentTask and
FailAgentTask now force session_id NULL (overriding Fail's COALESCE that
would keep a stale mid-flight pin) and set the flag in the SAME UPDATE, so
the withhold + gap flag commit atomically with the retry creation. The
flag is threaded through TaskService.CompleteTask/FailTask; the swallowed
best-effort MarkTaskSessionRolloutMissing query is removed.

Must-fix 2 — the daemon withholds for all Codex tasks, but only the issue
non-rerun claim consumed the disclosure. Now every fallback path sets
prior_session_resume_unavailable: the manual-rerun branch reads the source
task's session_rollout_missing, and the chat branch reads a new
GetLatestChatTaskRolloutMissing.

Tests (cross-layer DB):
- completed + rollout missing via the real CompleteAgentTask terminal
  write -> session withheld AND gap flagged
- failed + rollout missing forces session_id NULL over the COALESCE-
  preserved mid-flight pin in ONE statement

Deploy order: migration + server first, daemon second (new fields are
omitempty and ignored by an old peer).

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

* fix(handler): return 5xx on FailTask error + cover claim-response gap paths (MUL-5305)

Addresses review round 4 of #5960.

Must-fix 1 — the FailTask handler returned 400 on a service/DB error, but
the daemon's terminal callback treats 400 as permanent (postJSONWithRetry
/ isTransientError bails without retrying). Since the fail transaction is
now the sole persistence point for the withheld session + continuity-gap
flag + auto-retry, a rolled-back fail must be retried, so return 5xx (an
invalid request body still returns 400), mirroring CompleteTask.
Regression: client.FailTask retries on a transient 5xx and eventually
succeeds.

Must-fix 2 — add claim-response-level regressions that drive the two new
disclosure branches through buildClaimedTaskResponse:
  - chat: the latest terminal task on the session withheld -> the next
    chat claim sets prior_session_resume_unavailable
  - manual rerun: the source task withheld -> the rerun claim discloses
These handler DB tests run under CI's fully-migrated database (the local
workspace DB cannot set up the handler fixture).

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-27 13:12:06 +08:00
..