fix(workflow): clear resume token when reset_agent_context fires without live execution

[yattdev]

Problem

When a workflow step's on_enter declares reset_agent_context but no in-memory agent execution exists for the session (e.g. the session completed/stopped and is being lazily resumed), the reset was silently skipped. The early return omitted all persisted state clearing — the ACP session ID, resume token on the executors_running row, and context window metadata all survived.

The lazy resume path (applyRunningRecordToResumeRequest) reads the resume token directly from the persisted row, so a stale token would let the next lazy launch reconnect to the pre-reset ACP conversation. The agent would start its first turn with old, stale context — effectively ignoring the reset.

Fix

• Extracted clearResumeToken out of clearPersistedResetState and call it BEFORE ResetAgentContext in the live-execution path. The new ACP session does not exist yet, so the async session.created event (which calls storeResumeToken) cannot race with the clear — eliminating the store-before-clear interleaving.

• For the no-in-memory-execution path (lazy resume), clearResumeToken is called separately before the unchanged clearPersistedResetState. There is no ResetAgentContext call on this path, so no async event exists to race with.

• clearPersistedResetState now handles only ACP session metadata and context-window cleanup, without touching the resume token. This prevents it from erasing a fresh token written by the concurrent storeResumeToken on the live-execution path.

• A pre-existing concern remains: a stale ACP session.created event from the OLD session (same execution ID, old ACP session ID) can overwrite the fresh token via the execution-ID CAS, since ResetSession does not rotate the execution identity. The old code had the same window for acp_session_id metadata. Full fix requires a generation counter on executors_running (outside this scope).

Regression tests

• TestProcessOnEnterResetAgentContext_ClearsLazyResumeTokenWithoutLiveExecution — seeds a session with a stopped executor row carrying a resume token, triggers processOnEnter with a step that declares resetAgentContext, and asserts the resume token is cleared. (Original regression test, unchanged.)

• TestResetAgentContext_InterleavingA_StoreBeforeClear — simulates the dangerous interleaving where storeResumeToken fires before clearResumeToken. With the fix (clear before reset), the fresh token survives. PASS.

• TestResetAgentContext_InterleavingB_ClearBeforeStore — the favorable interleaving: clear runs before the async event writes the fresh token. PASS.

• TestResetAgentContext_InterleavingC_StaleOldEventOverwritesFresh — demonstrates the pre-existing concern where a stale old-session event overwrites the fresh token via the same execution-ID CAS. SKIP (pre-existing, requires generation counter).

Evidence finding — Review↔QA loop

Session 8c401f9f (task 6e0fc028) completed at 04:26:31 UTC and was re-entered at ~04:54. The observed already_signaled at 04:56:06 is mechanistically consistent with a stale agent context — the agent would see old conversation history and loop rather than proceeding. However, direct production evidence does not prove that the re-entry crossed a reset_agent_context on_enter with no live execution: there is no saved record of the specific branch, execution state, or workflow step that triggered the re-entry. The mechanism fixed here is a plausible downstream cause for the loop, but the practical observation cannot be definitively attributed to this bug without that execution trace. The terminal-session revival path (which shares the same no-in-memory-execution gap for a different entry point) is independently fixed by PR #2766.

Validation

• New regression test passes
• All existing orchestrator tests pass (go test ./internal/orchestrator/...)
• All pre-commit hooks pass (Conventional Commits, gofmt, Go lint, etc.)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: a1599bc6-969a-4eaa-9b1e-2c59cbb9745b

📥 Commits

Reviewing files that changed from the base of the PR and between 5eb99d8 and b95b75d.

📒 Files selected for processing (2)

• apps/backend/internal/orchestrator/event_handlers_workflow.go
• apps/backend/internal/orchestrator/event_handlers_workflow_lazy_resume_test.go

Included review availability: Your plan includes up to 4 reviews per rolling hour; 3 remain after this review.

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Fixed agent context resets during deferred or lazy resume.
  • Prevented previous conversations from being restored after a reset when no active agent execution exists.

• Tests

  • Added coverage verifying that reset actions clear deferred resume state without requiring an active agent process.

Walkthrough

The workflow reset path now clears persisted ACP session, resume-token, and context-window state when no live agent execution exists. A test verifies that reset-on-enter removes a stopped executor’s lazy-resume token without restarting the executor.

Changes

Agent context reset

Layer / File(s)	Summary
Clear reset state for deferred executions apps/backend/internal/orchestrator/event_handlers_workflow.go, apps/backend/internal/orchestrator/event_handlers_workflow_lazy_resume_test.go	The reset path clears persisted state for live and deferred executions. The test verifies lazy-resume token removal without live-process restart.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to b95b7

This localized workflow fix clears stale persisted resume state during agent-context resets, including lazy resumes without a live execution. The change is merge-ready after normal checks, with no actionable merge-blocking risk remaining.

Possibly related PRs

• kdlbs/kandev#1979: Modifies orchestrator session-resume behavior in a related area.
• kdlbs/kandev#2108: Also updates resetAgentContext to clear persisted reset state.
• kdlbs/kandev#2263: Also updates resetAgentContext for session reset handling.

Suggested reviewers: carlosflorencio, jcfs, zeval

Poem

A rabbit found a token tucked away,
And cleared it before the hop of day.
No sleeping agent woke,
No stale context spoke,
The reset path now starts fresh each way.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise, specific, and accurately describes the primary fix for clearing the resume token during reset_agent_context handling.
Description check	✅ Passed	The description clearly explains the problem, fix, regression tests, and validation, but it omits the template’s required summary prose and checklist.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Claude finished @yattdev's task in 1s —— View job

---

I'll analyze this and get back to you.

[greptile-apps]

Greptile Summary

The PR makes workflow context resets clear persisted recovery state even when no live agent execution exists.

• Extracts shared clearing of the ACP session ID, executor resume token, and context-window state.
• Applies the clearing to both live-execution and lazy-resume paths.
• Adds a regression test proving that a stopped executor's stale resume token is removed before its next launch.

Confidence Score: 5/5

The PR appears safe to merge, with the stale lazy-resume state cleared consistently across both reset paths.

The changed path preserves live-provider reset ordering, clears the persisted token consumed by lazy recovery, and adds a regression test for the previously skipped no-live-execution case without establishing any new blocking failure.

Important Files Changed

Filename	Overview
apps/backend/internal/orchestrator/event_handlers_workflow.go	Consolidates durable reset-state cleanup and invokes it on both successful live resets and confirmed no-live-execution resets.
apps/backend/internal/orchestrator/event_handlers_workflow_lazy_resume_test.go	Adds focused regression coverage proving that a no-live-execution workflow reset clears the persisted lazy-resume token.

Sequence Diagram

sequenceDiagram
  participant W as Workflow on_enter
  participant O as Orchestrator
  participant M as Agent manager
  participant DB as Persisted session state
  participant R as Lazy resume
  W->>O: reset_agent_context
  O->>M: Lookup live execution
  alt Live execution exists
    O->>M: Reset provider context
    M-->>O: Reset succeeded
  else No live execution
    M-->>O: No execution
  end
  O->>DB: Clear ACP session ID
  O->>DB: Clear resume token
  O->>DB: Clear context window
  R->>DB: Read recovery state
  DB-->>R: No stale resume token
  R->>R: Start a fresh conversation

Loading

_{Reviews (1): Last reviewed commit: "fix(workflow): clear resume token when r..." | Re-trigger Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b95b75df68

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[carlosflorencio]

Thanks for the contribution. I pushed 225e34598 with a few reliability improvements around context resets:

• reset and lazy resume now serialize per session;
• token-clear failures are returned instead of ignored;
• failed provider resets keep the existing recovery token;
• the fresh ACP token is persisted immediately, and delayed events from an old ACP session are ignored.

The focused orchestrator tests pass.