Review Import Additions for Multiple Agents

Codex·GPT-5.5·computermode·3mo ago·20min·1 Checkpoint·15 file changes·+1679·279.5K tokens

Review the current branch changes and report actionable findings. Prioritize correctness, regressions, security, and missing test coverage. Do not make code changes.

Review profile: general

Task: Review this change for correctness, regressions, API design, missing tests, maintainability, and user-facing behavior changes. Return only real, actionable defects with concrete evidence and an exact code pointer. No praise, summaries, speculation, style preferences, or nice-to-have refactors.

Output format:

  • Start with one verdict line: approve / approve with nits / request changes, plus a short reason.
  • Then list actionable findings only. Each finding MUST be a separate top-level Markdown bullet starting with [high], [medium], or [low].
  • Include an exact file:line pointer in each finding when possible, plus the bug, impact, and fix in one concise paragraph.
  • Do not combine multiple defects in one bullet or paragraph. Do not emit severity-heading paragraphs like "[HIGH] ..." without a leading bullet.
  • If there are no actionable findings, output only the verdict line.

Scope: review the commits unique to this branch vs origin/HEAD, plus any uncommitted changes in the working tree. Ignore code outside this scope.

In-progress session context (uncommitted): 65bd6f32 Claude Code (touched: 16 files) prompt: create test data in /Users/ninawork/entire/devenv/importtest for me to import with sample agent sessions to import

3mo ago·7.0s

I’ll inspect the branch delta against origin/HEAD and the working tree, then trace the changed code enough to identify only concrete defects in scope. No edits will be made.

There are no committed changes unique to this branch; the review scope is the working tree. I’m narrowing first on the modified agentimport package and the new provider files/tests, then I’ll decide whether any untracked support artifacts affect shipped behavior.

The shipped code change is a new set of importers wired into the registry, with tests beside them. I’m reading the importer implementations against the shared Run contract and checkpoint metadata shape now.

The untracked files include many local/plugin artifacts, but the only untracked Go code is under cmd/entire/cli/agentimport. I’m treating those new importers as in-scope because they are wired into the tracked registry change.

I’m checking the existing agent implementations now, because the importers delegate path discovery and token parsing to those packages. The main risk is not syntax; it’s mismatches between actual transcript formats and the new import adapters.

I’ve found that the registry change makes these importers reachable through the existing hidden entire import command. Next I’m checking command output and checkpoint read/write assumptions, especially where JSON vs JSONL transcripts pass through shared storage.

I haven’t found review-worthy defects yet; the likely issue areas are now down to format handling and persisted metadata semantics. I’m going to run the focused package tests as another signal, then inspect the exact transcript helpers where the new code relies on them.

One concrete risk is emerging: Run always sends transcript bytes through a JSONL redactor, while Gemini imports JSON documents. I’m verifying whether that path actually fails or silently corrupts before I report it.

The redactor does accept a complete JSON document before falling back to line-by-line JSONL, so the Gemini redaction concern is not a defect on its own. I’m continuing through per-agent format adapters for errors that would produce bad imports or unusable history.

The focused package tests pass. I’m doing one compile/test pass on the command package next because the registry affects Cobra command wiring and help/tests outside agentimport.

I found one real defect: the Cursor importer assumes a uuid field that the existing Cursor fixture for real transcripts does not have, which collapses multiple turns onto the same checkpoint ID. I’m checking the same stable-ID assumption across the other new importers.

The stable-ID check shows the UUID problem is specific to Cursor; the other importers either read IDs present in their agent format or synthesize one. I’m now looking for repo attribution mistakes in global-session importers like Codex and Copilot.

A second defect is likely in Copilot discovery: the new importer filters transcripts by fields that the existing Copilot transcript model and fixtures do not include. I’m checking whether any alternate code path persists that context before treating it as a real missed-import bug.

Copilot import has a real mismatch: existing Copilot “real format” fixtures store session.start without cwd/gitRoot, but the new importer requires data.context.cwd or data.context.gitRoot, so discovery returns nothing. I’m checking Codex and Gemini discovery against their existing agent tests for the same class of mismatch.

Factory and Cursor do not populate CreatedAt when their transcript lines lack timestamps, which affects imported checkpoint ordering. I’m verifying whether that rises to an actionable finding or is already handled by the checkpoint layer.

There are also many untracked non-code artifacts in the working tree. I’m checking only whether any of those would create a correctness or security issue if they are part of the submitted change; otherwise I’ll keep the findings focused on the importer code.

request changes - Cursor and Copilot importers do not handle the repo’s documented real transcript shapes.

  • [medium] cmd/entire/cli/agentimport/cursor.go:117 uses rec.UUID as the turn key, but the existing real-format Cursor fixture has no uuid field (cmd/entire/cli/agent/cursor/cursor_test.go:19). All turns in such a session get UUID == "", so Run derives the same checkpoint ID and skips every turn after the first as already imported. Fix by falling back to a stable per-turn key such as the line index when uuid is empty, and add a uuid-less Cursor import test.

  • [medium] cmd/entire/cli/agentimport/copilot.go:93 only accepts Copilot sessions whose session.start has data.context.cwd or data.context.gitRoot, but the repo’s real-format Copilot fixture stores session.start without that context (cmd/entire/cli/agent/copilotcli/copilotcli_test.go:17). As a result, discovery returns no real Copilot sessions, even with an explicit session directory. Fix discovery to use fields that actually exist in Copilot’s stored transcript, or provide a supported explicit-session path that bypasses repo attribution safely.