Preserve session imports and migrate plugin commands#31483
Preserve session imports and migrate plugin commands#31483charlesgong-openai wants to merge 1 commit into
Conversation
![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: 861790e1aa
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| body | ||
| }; | ||
| format!( | ||
| "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", |
There was a problem hiding this comment.
When a plugin command has a large Markdown body, this renders the entire command template into a generated SKILL.md; the skill injection path later reads the full skill file into model context, so installing such a plugin can create a new context item well over the required 1k/10k-token caps. Please bound, truncate, or skip oversized command bodies before writing the generated skill.
AGENTS.md reference: AGENTS.md:L91-L99
Useful? React with 👍 / 👎.
| let migrated_command_skills = migrated_command_skills_root(plugin_root); | ||
| if migrated_command_skills.is_dir() { | ||
| paths.push(migrated_command_skills); |
There was a problem hiding this comment.
For plugins that were already installed before this change, .codex-plugin/migrated-command-skills will not exist because migration only runs from PluginStore::install; this load path just skips the missing generated root, so supported commands in existing plugin caches remain invisible until the user reinstalls the plugin. Run the command migration before computing skill roots, or add a startup/load-time backfill for cached plugin roots.
Useful? React with 👍 / 👎.
| import_commands( | ||
| &plugin_root.join(PLUGIN_COMMANDS_DIR), |
There was a problem hiding this comment.
For marketplace-installed plugins that declare command files outside the literal top-level commands/ directory (for example "commands": ["./tools/review.md"] in the manifest fallback), fresh installs still complete but this scans only plugin_root/commands, so those supported commands never get generated as skills. Use the declared command paths, or include them in addition to the conventional directory.
Useful? React with 👍 / 👎.
| let Ok(Some(summary)) = summarize_session(&path) else { | ||
| continue; | ||
| }; | ||
| match ledger.refresh_current_source( |
There was a problem hiding this comment.
When the 50 newest .jsonl files are moved or modified sessions that were already imported under the same source session ID, they all enter file_candidates before this later identity check drops them, so older unimported sessions are never considered in that detect pass. This regresses discovery for users with many recent already-imported/renamed source sessions; parse or overfetch enough candidates before applying the SESSION_IMPORT_MAX_COUNT cap.
Useful? React with 👍 / 👎.
1 participant
Summary
codex-core-pluginsRoot cause
Session imports were keyed by source path and content hash, so moving or changing an already imported source could create a duplicate. Imported rollout metadata was also generated at import time, and later rollout reconciliation could overwrite the source chronology even when SQLite had briefly received the correct timestamps.
Command migration only ran through the external-agent migration path. Installed plugins could therefore contain supported commands without exposing the corresponding generated skills.
Impact
Imported sessions remain associated with their original project and sort using their source chronology. Re-imports use the stable source session identity, and concurrent requests reserve that identity before work starts. Supported commands from installed plugins are available through the generated skill root.
Validation
just bazel-lock-updatejust test -p codex-core-plugins load_plugins_includes_migrated_command_skills_with_explicit_skill_pathsjust test -p codex-app-server external_agent_config_import_creates_session_rolloutsjust test -p codex-rollout -p codex-thread-store -p codex-external-agent-migration -p codex-external-agent-sessions -p codex-state(369 passed)just fix -p codex-app-server -p codex-core-plugins -p codex-external-agent-migration -p codex-external-agent-sessions -p codex-rollout -p codex-state -p codex-thread-storejust fmtAn earlier broad app-server run also exercised the affected paths, but unrelated command-exec and executor tests cannot run under the current macOS sandbox, and one plugin test observed home-level imported skills. The focused regressions and affected non-app-server crate suites above pass.
Change size
The diff exceeds the usual 800-line review guideline. About 287 lines are removed from
codex-external-agent-migrationand replaced by the reusable 348-linecommand_migration.rsmodule incodex-core-plugins; the other coherent workstream is session import fidelity and deduplication. These could be staged as separate PRs, but this draft intentionally keeps all requested workspace changes together.