refactor(agent-core-v2): compose the llm recovery chain outside the turn machine - #3724
Conversation
…urn machine The turn machine hardcoded credentialsRecovery as the head of its recovery chain and interpreted a refreshCredentials flag on proposals, coupling the turn to the credentials domain. Strategies now return self-describing records (strategy/action, optional messages, optional opaque prepare effect), the turn only runs prepare and/or swaps messages, and the engine composes the chain via chainRecoveries.
|
|
@codex review |
commit: |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
With self-describing proposals the chain is a one-liner at each call site; the named chainRecoveries combinator had only two call sites and no third strategy to justify it.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Related Issue
N/A — internal refactor spun out of a design review of #3682.
Problem
The turn machine was coupled to the credentials domain in two places: it imported
credentialsRecoveryand hardcoded it as the head of the recovery chain, and it interpreted a credential-namedrefreshCredentialsflag on recovery proposals by callingcredentials.invalidate()itself. Recovery orchestration belongs to the turn; credential knowledge does not.What changed
LlmRecovery.proposenow returns a self-describing record (strategy/action, optional replacementmessages, optional opaqueprepareeffect). The credential-specificrefreshCredentialsflag is gone, and the now-unusedLlmRecovery.idgoes with it.credentialsRecoveryencapsulates the refresh in aprepareclosure;createMediaDegradeRecoverystamps its ownstrategyid.proposal.prepare?.()and/or swaps messages.turn.failure.triagedsemantics are unchanged.chainRecoveries(...)combinator; the engine composeschainRecoveries(credentialsRecovery, options.recovery), keeping the "refresh credentials before degrading media" priority at the composition root.docs/en\|zh/llm.md) updated to match.Behavior is identical: same event sequence, same
llm.recoveringrecords, same per-turn strategy dedup. Verified with typecheck, the full package vitest run (6562 tests), oxlint, and the repo no-comments / import-boundary checks. No changeset: internal refactor, not user-perceivable.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.