fix(agent-core-v2): align compaction injections and goal tools with v1 - #1602
Conversation
🦋 Changeset detectedLatest commit: fb62b36 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c036994fed
ℹ️ 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".
| if (previousMode === undefined && injectedPositions.length > 0) { | ||
| this.lastMode = currentMode; | ||
| return undefined; |
There was a problem hiding this comment.
This adopts any surviving permission_mode injection solely from its position, but the injector tracks only the variant, not whether the live message is the auto-enter or auto-exit reminder. If a session has an exit reminder in history, the user switches back to auto, and the process restores before the next injection, this branch seeds lastMode as auto and suppresses AUTO_MODE_ENTER_REMINDER; the model is left with stale “auto no longer active” context while auto approvals are active. Only adopt restored history when the surviving reminder actually matches the current mode, or re-announce auto in this case.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| registerTool(CreateGoalTool); | ||
| // Goal tools are main-agent-only (v1 parity: `agent.type === 'main'`). |
There was a problem hiding this comment.
packages/agent-core-v2/AGENTS.md says comments in this package must live solely in the top-of-file /** */ block and never beside statements, but this added // Goal tools are main-agent-only... comment sits next to the registerTool statement (and the same pattern was added to the other goal tool modules). Please move this rationale into the header comment or let the guarded registration stand without the inline comment.
Useful? React with 👍 / 👎.
- Re-announce the auto permission mode reminder after compaction by deriving dedup from the reminder's live history positions (also self-heals undo and session restore). - Render goal reminders byte-identical to v1 by inlining nunjucks block tags that previously left blank lines around optional sections. - Match v1's background-task reminder guidance text exactly. - Gate the goal tools to the main agent via a `when` contribution guard, as v1 does with agent.type === 'main'.
c036994 to
b85fada
Compare
commit: |
…, header-only comments - Drop the silent-adoption of surviving reminders on fresh instances: a stale exit reminder in restored history could suppress the auto-mode announcement. A fresh instance now announces the current mode exactly as v1 does. - Move the main-agent-only rationale for goal tool registration into the file-header comment blocks per the package comment convention.
|
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". |
1 participant
Related Issue
No linked issue — the problem is explained below (found during a v1/v2 parity audit of the full-compaction path).
Problem
agent-core-v2's full-compaction path diverged from v1 (agent-core) in several model-visible ways:lastMode; once compaction spliced the reminder out of history it was never re-announced (v1 re-arms viaonContextCompacted).trimBlocks, so block-tag lines around optional sections (completion criterion, budgets, guidance band) leaked blank lines; v1 builds the same text inline.a task resultvsa task’s result).agent.type === 'main'.What changed
PermissionModeInjectionnow derives dedup from the reminder's live positions in history (the same "history is the ledger" patternplan_modeand loadable-tools use): the reminder is re-announced after compaction/undo, and a fresh instance after session restore announces the current mode exactly as v1 does — without inspecting surviving reminder content, so a stale exit reminder can never suppress the auto-mode announcement.whenguard to the four goal tool contributions.agent-core-v2suite passes; typecheck clean.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update — v2-internal parity fixes; documented CLI (v1) behavior is unchanged.