refactor(agent-core-v2): unify model-facing reminder scheduling - #2623
Open
7Sageer wants to merge 28 commits into
Open
refactor(agent-core-v2): unify model-facing reminder scheduling#26237Sageer wants to merge 28 commits into
7Sageer wants to merge 28 commits into
Conversation
Route every model-facing reminder through the contextInjector boundary scheduler. Past-tense events go through a persisted once-reminder queue (reminderQueue) that delivers exactly once at turn, step, compaction, and restore boundaries; present-tense state renders through context-injection providers reconciled against live history. - interruption, goal (cancel/budget/fork-cleared), image-compression captions, btw, and init reminders enqueue into reminderQueue instead of writing the context directly; the interruptionReminder wire model is removed and its recorded type is retired silently on replay - swarm mode announcements render through a provider seeded from the replayed history on restore, replacing live side effects and the ContextModel pop reducer on swarm_mode.exit - loadable-tools announcements become an isNewTurn-gated provider, dropping the compaction boundary flag - plugin session-start guidance re-renders as a supersedes reminder at the next boundary via a dirty flag instead of appending immediately - legacy system_trigger origins of migrated reminders still fold on replay
|
# Conflicts: # packages/agent-core-v2/src/agent/contextInjector/contextInjectorService.ts
commit: |
- drop the swarmInjection re-export from the package index; SwarmInjection stays a domain-internal collaborator like permissionMode/plan injections - move INTERRUPTION_REMINDER text back to a private constant in the service; only the variant stays in the Ops module - make reminderQueue.enqueue return void; no caller consumed the entry id
- derive swarm active/inactive state from ctx.lastDisclosure instead of byte-matching rendered markdown, with variant-only fallback for legacy swarm_mode/swarm_mode_exit journal entries - record once_reminder disclosure (entry id) on queue-appended messages and dedupe the crash window by the contiguous tail id set, covering multi-entry drains - move reminderQueue draining behind a sync onWillInject event so the injector no longer depends on the queue domain - centralize the system-reminder wrap format behind wrapSystemReminder / systemReminderContent and use injector-provided positions in the plugin session-start provider - spell out the step-boundary fallback and sync-only contract of registerAtTurnStart via shouldRunAtBoundary
…e per missing sessionStart skill
Drop the per-provider positions cache from the context injector: the registration scan, the context.spliced index arithmetic, and the post-restore resync all existed only to mirror what the history already records. Each provider call now derives its injected positions by scanning context memory for its surviving injection messages, so silent history edits (such as vacuous-step folds) can no longer desync a cached index.
…undaries Move the dynamic-tool schema declaration out of toolSelect.load(): the loaded names are recorded as pending and drained by a dedicated toolSelectSchemas provider through the contextInjector boundary scheduler, so the declaration message lands at a quiescent boundary instead of mid-step inside a streaming tool exchange. The folded history remains the loaded-tool ledger, so undo, compaction, and resume still self-heal by re-folding.
…nder queue The tool hook now only observes and enqueues a once-per-agent reminder through the reminderQueue once-channel instead of prepending text to the tool result: results stay verbatim for the truncation pipeline and the reminder can never be truncated away with an oversized output. The reminderQueue is resolved lazily through the instantiation service at enqueue time, breaking the contextInjector -> loop -> llmRequester -> profile -> agentsMdReminder constructor cycle.
…-owned contextMemory no longer declares the ContextInjectionDisclosure union: InjectionOrigin.disclosure becomes an opaque unknown, and providers bind their own payload type through register<D>, so lastDisclosure arrives at the provider already typed by its own variant. The date, swarm_mode, and once_reminder payload shapes move into the dateChange, swarm, and reminderQueue domains respectively; reminderQueue keeps a runtime guard for its cross-message tail scan, the only place that reads disclosures it did not write. Persisted origin shapes are byte-identical, so existing journals replay unchanged.
A step or compaction boundary provider that threw or rejected made the injector's inject() promise reject, which propagated through the onWillBeginStep hook chain and failed the whole turn, and starved every provider registered after it. Log and skip the bad provider instead, matching the turn-start path's existing isolation.
…dary Replace the shared read-and-clear isNewTurn flag (set by turn.started and injectAfterCompaction, consumed by the first inject()) with values each trigger supplies from an authoritative source: the loop marks a turn's first step via BeforeStepContext.firstStepOfTurn (standalone runs never count), and the compaction follow-up passes true explicitly, so interleaved triggers can no longer consume or steal the marker. A compaction follow-up that lands inside a step hook chain (the auto-compaction path) doubles as that step's new-turn delivery: the enclosing step then injects with isNewTurn false, so the upcoming request receives one new-turn injection, not two.
A load announced by select_tools sits in pendingLoaded until the next injection boundary declares it. A compaction fold in that window publishes a replacement splice, and the splice-time reconciliation dropped the pending entries before the post-compaction inject could declare them — the model was told "Loaded: X" yet X never became available. Drop pending entries only on removal splices (undo/clear, which carry no replacement messages); compaction's replacement splice keeps them so the declaration lands at the post-compaction boundary.
…successful render reconcileSessionStartReminder cleared the refresh-pending flag before awaiting the render, so a throwing render (skipped by the injector's provider isolation) lost the forced refresh until the next catalog change. Consume the flag only after the render resolves, and move the warn-once rationale into the module header per the comment convention.
# Conflicts: # packages/agent-core-v2/src/agent/plugin/agentPluginService.ts # packages/agent-core-v2/src/features/btw/btwService.ts # packages/agent-core-v2/test/agent/plugin/agentPlugin.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
No linked issue — this is an internal engine refactor; the problem is explained below.
Problem
Model-facing reminders in agent-core-v2 were delivered ad hoc: six domains (interruption notices, goal cancel/budget/fork-cleared, image-compression captions, btw, init) computed their own delivery timing and wrote the conversation directly through the system-reminder service, each re-implementing crash recovery, duplicate suppression, and boundary ordering. Swarm mode additionally relied on a cross-model context reducer to pop its enter reminder on exit, and tool-load announcements kept a dedicated compaction-boundary flag. Delivery semantics were subtly different per reminder kind and hard to reason about (immediate append vs. step boundary vs. restore reconcile).
What changed
Reminders now take exactly two sanctioned paths, chosen by fact shape (also codified in
packages/agent-core-v2/AGENTS.md):IAgentSystemReminderService.appendSystemReminder, stamped with aninjectionorigin ({ kind: 'injection', variant: '<domain_fact>' }). Goal cancel / budget-stop / fork-cleared,/initcompletion, and the btw side-channel reminder (into the forked child) all append this way. The AGENTS.md reminder no longer prepends into the tool result (which forced head-insertion against truncation and hook ordering beforetoolDedupe) — it appends a standalone<system-reminder>message at the discovery point instead, letting the domain service shed those guards (net −76 lines). Image-compression captions append in the prompt/steer step request'sonWillMaterialize, ahead of the host message and carrying itsownerPromptId, so undo rewinds them together with the host prompt. Producers keep their own once-guards where re-delivery is possible (theagentsMdReminder.knownstate set, the fork-notice model'sreminderPendingflag); a failed AGENTS.md append simply retries on the next qualifying touch.contextInjectorproviders, reconciled against the live history at every injection boundary (turn start, step, compaction follow-up): swarm-mode announcements move to a provider seeded from the replayed history on restore (replacing live side effects and theContextModelpop reducer onswarm_mode.exit), loadable-tools announcements become anisNewTurn-gated provider (dropping the compaction-boundary flag — the folded history is the ledger, so undo/compaction/resume self-heal by re-folding), and plugin session-start guidance re-renders as a supersedes reminder at the next boundary via a dirty flag.turn.cancelfact into pending reminder state — so the pending set follows conversation undo through the checkpoint protocol — and a turn-start provider renders a single notice for the pending cancellations, collapsing retry-only duplicates by reading the conversation tail; the model clears its pending state when the injection lands.Supporting injector changes:
context.splicedindex arithmetic, post-restore resync) and computes each provider's surviving injection positions by scanning the conversation when the provider runs. The public provider contract (injectedPositions/lastInjectedAt) is unchanged, and silent history edits (such as vacuous-step folds that fire no splice event) can no longer desync a cached index.<system-reminder>messages, content parts append as bare user messages, and tagged raw messages append verbatim with the injection origin stamped — the form the dynamic-tool schema declaration now uses for itstoolsfield.kind: 'injection'becomes a lifecycle classification, not a provenance claim: injection-origin messages are hidden from the UI, are not undo anchors, and are dropped by compaction.Compatibility details: legacy
system_triggerorigins of migrated reminders still fold on replay (loadable-tools announcements, goal fork-cleared); theinterruptionReminder.recordedop keeps its name and shape, so pre-refactor journals replay without unknown-record reports, and on resume a cancellation whose delivery never landed re-renders exactly once while a recorded delivery stays silent.Verified with the full agent-core-v2 suite (298 files, 4730 tests), including new coverage for resume-time swarm seeding, legacy-origin folding, legacy interruption reconciliation, and injector position/disclosure derivation. No user-facing behavior change on shipped surfaces (v2 engine internals), so no changeset per the gen-changesets rules.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.