Skip to content

fix(acp): keep SDK prompt owner across session continuations - #6354

Merged
Yeachan-Heo merged 2 commits into
devfrom
gjc-abandon-agentend-corr
Oct 5, 2026
Merged

Yeachan-Heo merged 2 commits into
devfrom
gjc-abandon-agentend-corr

Conversation

@probepark

@probepark probepark commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Keep the causal SDK prompt owner when AgentSession.#scheduleAgentContinue schedules a continuation without an explicit token. The shared producer now captures #activeSdkRunToken (or the active attempt-scope token) at schedule time and avoids clearing a defined owner during inherited continuations.

Field evidence from tank / gjc 0.18.7 (2026-10-05): 61 incomplete_correlation drops, each followed by watchdog_expired 340s later; 40 followed provider retry only, 7 had no retry (todo reminder path), and ~13 were async-result + retry.

Test: injects a continuation for an interactive todo reminder — GREEN after restore. The required discriminating SDK-host lifecycle assertion could not be reached through the current AgentSession test harness without bypassing the production SDK admission path; the focused reminder test proves continuation injection but does not distinguish owner propagation.

Acceptance

ID Result
AC-1 Continuation agent_end carries original commandId/turnId via shared owner capture; todo-reminder producer path covered by continuation regression test.
AC-2 Existing retry path preserved; focused retry recovery tests pass.
AC-3 No ACP validation, watchdog bounds, retry counts, or timeout changes.

Verification

  • bun test packages/coding-agent/test/agent-session-todo-reminder.test.ts -t 'injects a continuation for an interactive todo reminder' — pass
  • bun test packages/coding-agent/test/agent-session-retry-busy-recovery.test.ts packages/coding-agent/test/agent-session-todo-reminder.test.ts — 13 pass
  • bun run --workspaces --if-present check:types — rc 0
  • bun run lint — rc 0
  • RED proof: attempted with only #scheduleAgentContinue fix reverted; current test remained green because it does not observe SDK correlation, so this is blocked.

Provider-error retries can start after the failed attempt retires its active SDK scope. Capture and propagate the original run token so terminal agent events settle the owning prompt.
Keep the captured SDK token in session ownership metadata instead of passing it to AgentPromptOptions, which does not expose that internal field.

@snowykr snowykr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

APPROVED

Summary

This change snapshots the failing SDK attempt’s run token before scope retirement and carries it through both direct and scheduled provider-error retry continuations. The token stays within the existing session ownership and scheduling contracts, while ACP’s complete-correlation guard continues to reject foreign terminals. No merge-blocking defects were identified.

Findings / Required Changes

No blocking or actionable findings.

Non-blocking Observations

  • A targeted regression test combining provider-error retry after agent_end/scope retirement with asserting final ACP prompt correlation would cover the remaining test gap. Existing retry/busy-recovery and terminal-correlation tests cover adjacent behavior, but not their combination. This is an optional coverage improvement, not a demonstrated defect.

CI / Verification

For reviewed head 60e1f36eead1e4d545af767816a3a423a84978bf, the completed GitHub checks included Affected path validation, Affected path validation / evidence producer, Virtual integration validation, and gjc-state-gates; the exact-SHA CI summary also showed the affected-path test matrix and coding-agent package/type checks passing. The combined commit status was pending with no status contexts. No tests or builds were run as part of this review.

Axis Coverage

Axis Verdict Coverage
A1 — Intent / Policy / Contract APPROVED Captured retry token propagation follows existing SDK/ACP correlation ownership contracts; no persisted/public contract changed.
A2 — Architecture / Correctness / Failure APPROVED Checked retry acceptance/scheduling, scope lifecycle, cleanup, duplicate/foreign terminal guards, and failure ordering; no reachable regression found.
A3 — Security / Privacy / Trust APPROVED Token is captured from internal attempt ownership rather than untrusted caller data; no new authority or trust boundary crossing.
A4 — Verification / Tests / CI APPROVED Adjacent tests and exact-head successful checks reviewed; missing end-to-end correlation/retry combination is optional coverage only.
A5 — Context / Compatibility / Platform APPROVED Existing scheduler/acceptance and ACP consumer contracts remain intact; no new persistence, platform, or packaging surface and no material duplicate abstraction.

Limitations

The combined commit status remained pending without status contexts, although the visible exact-head check runs were successful. Local verification mentioned in the PR description was not independently reproduced.

@Yeachan-Heo
Yeachan-Heo merged commit 380f4aa into dev Oct 5, 2026
47 checks passed
@Yeachan-Heo

Copy link
Copy Markdown
Owner

Merged into dev.

  • snowykr approved the exact head 60e1f36.
  • CI on that head: 22 passed, 0 failed.
  • Because it changes src/session/agent-session.ts, I merged it locally with current dev and ran the 80 test files that import agent-session, on both the merged tree and unmodified dev. Only one test failed only on the merged tree: AgentSession context promotion > preserves larger-model headroom after promotion past the default 300K threshold. Run on its own, that file passed 3/3 on the merged tree and 2/2 on dev, so it's intermittent and not caused by this change.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

@probepark probepark changed the title fix(acp): type retry correlation propagation fix(acp): keep SDK prompt owner across session continuations Oct 5, 2026
@probepark

Copy link
Copy Markdown
Collaborator Author

gjc-acp-feedback: signature 20bc04f27d05 recurred — 6 occurrences in the last 7 days across 2 dev machine(s), gjc <ba25a3543eb4, ba25a3543eb4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants