Add explicit runtime migration tooling#37
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds explicit, lane-aware migration tooling: reporting, import with backups/manifest and rollback, CLI commands (audit/import/rollback), tests, docs, extends legacy importer to accept extra legacy paths, and removes implicit init-time copying of agent-facing state. ChangesRuntime Migration Feature
Sequence DiagramsequenceDiagram
participant CLI as aimux CLI
participant Migr as RuntimeMigration
participant Store as RuntimeExchangeStore
participant FS as Filesystem
CLI->>Migr: audit(projectRoot) -> buildRuntimeMigrationReport()
Migr->>FS: scan legacy dirs & validate files
Migr-->>CLI: report JSON
CLI->>Migr: import(projectRoot)
Migr->>FS: read legacy artifacts (history/context/status)
Migr->>Store: importRuntimeExchangeFromLegacyFiles(...)
Migr->>FS: write migration-backups/<timestamp>/runtime-exchange.yaml (backup)
Migr->>FS: copy legacy dirs -> .aimux/
Migr->>FS: write manifest.json
Migr->>Store: write runtime-exchange.yaml
Migr-->>CLI: import result JSON
CLI->>Migr: rollback(manifestPath)
Migr->>FS: restore backups, remove written files/dirs
Migr-->>CLI: rollback result JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/dev-runtime.md (1)
50-56: ⚡ Quick winAdd a rollback command example next to audit/import.
Lines 50-53 show the forward flow only; adding rollback usage here makes recovery workflow immediately discoverable.
📘 Suggested doc update
```sh aimux-dev migration audit --project /path/to/scratch-project aimux-dev migration import --project /path/to/scratch-project +aimux-dev migration rollback <manifest-path></details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/dev-runtime.mdaround lines 50 - 56, Add a rollback example next to the
existing migration audit/import snippets by inserting the command usage
"aimux-dev migration rollback " immediately after the import line
and include a short one-line note that this command restores state from the
recorded rollback manifest (created under migration-backups/) so readers can
discover the recovery workflow alongside audit/import; reference the command
name "aimux-dev migration rollback" and the manifest concept so the addition is
easy to locate and verify.</details> </blockquote></details> <details> <summary>src/runtime-core/exchange-import.ts (1)</summary><blockquote> `353-365`: _⚡ Quick win_ **Deduplicate and validate additional paths before building continuity refs.** Line 353 onward appends raw `additional*Paths`; this can introduce duplicate/blank/non-existent paths, which then produce duplicate or malformed continuity refs. <details> <summary>Proposed hardening (dedupe + trim + existence + extension checks)</summary> ```diff +function mergeContinuityPaths( + discovered: string[], + additional: string[] | undefined, + isAllowed: (path: string) => boolean, +): string[] { + return unique([...discovered, ...(additional ?? [])]).filter((path) => existsSync(path) && isAllowed(path)); +} + export function importRuntimeExchangeFromLegacyFiles( input: { @@ return buildRuntimeExchangeFromLegacySnapshot({ @@ - historyPaths: [ - ...listFiles(getHistoryDir(), (name) => name.endsWith(".jsonl")), - ...(input.additionalHistoryPaths ?? []), - ], - contextPaths: [ - ...listNestedFiles(getContextDir(), (name) => name.endsWith(".md") || name.endsWith(".jsonl")), - ...(input.additionalContextPaths ?? []), - ], - recordingPaths: [ - ...listFiles(getRecordingsDir(), (name) => name.endsWith(".txt") || name.endsWith(".log")), - ...(input.additionalRecordingPaths ?? []), - ], - statusPaths: [...listFiles(getStatusDir(), (name) => name.endsWith(".md")), ...(input.additionalStatusPaths ?? [])], + historyPaths: mergeContinuityPaths( + listFiles(getHistoryDir(), (name) => name.endsWith(".jsonl")), + input.additionalHistoryPaths, + (path) => path.endsWith(".jsonl"), + ), + contextPaths: mergeContinuityPaths( + listNestedFiles(getContextDir(), (name) => name.endsWith(".md") || name.endsWith(".jsonl")), + input.additionalContextPaths, + (path) => path.endsWith(".md") || path.endsWith(".jsonl"), + ), + recordingPaths: mergeContinuityPaths( + listFiles(getRecordingsDir(), (name) => name.endsWith(".txt") || name.endsWith(".log")), + input.additionalRecordingPaths, + (path) => path.endsWith(".txt") || path.endsWith(".log"), + ), + statusPaths: mergeContinuityPaths( + listFiles(getStatusDir(), (name) => name.endsWith(".md")), + input.additionalStatusPaths, + (path) => path.endsWith(".md"), + ), attachments: readLegacyAttachments(), }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime-core/exchange-import.ts` around lines 353 - 365, The code appends raw additional*Paths into historyPaths/contextPaths/recordingPaths/statusPaths which can introduce duplicates, blank entries or non-existent/malformed paths; update the construction of historyPaths, contextPaths, recordingPaths and statusPaths (the lines that use listFiles/listNestedFiles and input.additional*Paths) to first normalize each additional path by trimming whitespace, filter out empty strings, validate file existence (e.g. fs.existsSync) and enforce allowed extensions (.jsonl for history, .md|.jsonl for context, .txt|.log for recordings, .md for status), and deduplicate (e.g. via a Set) before spreading them into the arrays so only valid, unique paths are added.src/runtime-migration.ts (1)
458-460: 💤 Low valueDuplicate manifest write is redundant.
The manifest is written twice (lines 458 and 460) with identical content. The second write doesn't capture any state change since
manifestisn't modified between the two calls. If line 458 is meant as a checkpoint before the exchange write, it's misleading because it already records the exchange inmanifest.wrotebefore the actual write occurs.Consider removing line 458 and writing the manifest only after the exchange is successfully persisted.
♻️ Suggested fix
manifest.wrote.push({ kind: "runtime-exchange", path: report.authority.runtimeExchangePath }); - writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n"); const writtenExchange = new RuntimeExchangeStore(report.authority.runtimeExchangePath).write(exchange); writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime-migration.ts` around lines 458 - 460, The manifest is being written twice with no intervening changes; remove the first writeFileSync call and instead call writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n") only after the exchange has been successfully persisted by RuntimeExchangeStore(report.authority.runtimeExchangePath).write(exchange) (the writtenExchange result), so update the code around manifestPath/manifest and RuntimeExchangeStore.write to persist the exchange first and then write the manifest once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.ts`:
- Around line 3001-3007: The audit command is supposed to be read-only but
global preAction always calls initPaths(projectRoot) which mutates state; update
the preAction hook to skip calling initPaths(projectRoot) when the invoked
command is "audit" (or when opts indicate read-only), or add a readOnly flag
that prevents mutation and check it before calling initPaths; ensure the audit
action still calls resolveProjectRoot/pathResolve and then directly runs
buildRuntimeMigrationReport/renderRuntimeMigrationReport without triggering
initPaths so migration audit remains non-mutating.
---
Nitpick comments:
In `@docs/dev-runtime.md`:
- Around line 50-56: Add a rollback example next to the existing migration
audit/import snippets by inserting the command usage "aimux-dev migration
rollback <manifest-path>" immediately after the import line and include a short
one-line note that this command restores state from the recorded rollback
manifest (created under migration-backups/) so readers can discover the recovery
workflow alongside audit/import; reference the command name "aimux-dev migration
rollback" and the manifest concept so the addition is easy to locate and verify.
In `@src/runtime-core/exchange-import.ts`:
- Around line 353-365: The code appends raw additional*Paths into
historyPaths/contextPaths/recordingPaths/statusPaths which can introduce
duplicates, blank entries or non-existent/malformed paths; update the
construction of historyPaths, contextPaths, recordingPaths and statusPaths (the
lines that use listFiles/listNestedFiles and input.additional*Paths) to first
normalize each additional path by trimming whitespace, filter out empty strings,
validate file existence (e.g. fs.existsSync) and enforce allowed extensions
(.jsonl for history, .md|.jsonl for context, .txt|.log for recordings, .md for
status), and deduplicate (e.g. via a Set) before spreading them into the arrays
so only valid, unique paths are added.
In `@src/runtime-migration.ts`:
- Around line 458-460: The manifest is being written twice with no intervening
changes; remove the first writeFileSync call and instead call
writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n") only after
the exchange has been successfully persisted by
RuntimeExchangeStore(report.authority.runtimeExchangePath).write(exchange) (the
writtenExchange result), so update the code around manifestPath/manifest and
RuntimeExchangeStore.write to persist the exchange first and then write the
manifest once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 264a621d-4344-4448-a5ba-07434af9cf23
📒 Files selected for processing (7)
docs/dev-runtime.mddocs/runtime-authority-dead-paths.mdsrc/main.tssrc/paths.tssrc/runtime-core/exchange-import.tssrc/runtime-migration.test.tssrc/runtime-migration.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
1 participant
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.
Summary
Verification
Summary by CodeRabbit
New Features
aimux migrationcommands:audit(read-only inspection),import(imports legacy runtime artifacts into active lane, copies legacy artifacts into repo-local state, and records a timestamped rollback manifest), androllback(restores prior state using a manifest).Documentation
Refactor
Tests