Refine archive create/import TUI#102
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughThis PR replaces archive creation input handling with Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/archive.ts (1)
464-501: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTemp directory leaked if
DirectoryDocument.openthrows.In
createEmptyArchive,DirectoryDocument.open(directoryPath)runs before thetry/finallythat removesdirectoryPath. Ifopenthrows (e.g. disk/permission failure), the temp dir created bycreateWikiGraphTempDirectoryis never cleaned up.🐛 Proposed fix
async function createEmptyArchive(outputPath: string): Promise<void> { const directoryPath = await createWikiGraphTempDirectory("archive-write"); - const document = await DirectoryDocument.open(directoryPath); try { + const document = await DirectoryDocument.open(directoryPath); try { await document.openSession(async (openedDocument) => { await openedDocument.writeToc({ items: [], version: TOC_FILE_VERSION, }); }); } finally { await document.release(); } await writeWikgArchive(directoryPath, outputPath); } finally { await rm(directoryPath, { force: true, recursive: true }); } }🤖 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/cli/archive.ts` around lines 464 - 501, The temp directory cleanup in createEmptyArchive is only protected after DirectoryDocument.open, so a failure there can leak the directory created by createWikiGraphTempDirectory. Move the DirectoryDocument.open handling inside the existing cleanup try/finally (or add an outer finally around it) so directoryPath is always removed, and make sure document.release is still called only when open succeeds.
🤖 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 `@data/help/commands/maintenance/chapter/add.jinja`:
- Around line 10-11: The dedicated chapter add command usage text is missing the
newly supported --json flag. Update the add usage strings in the chapter add
template to include --json alongside the existing options for both stage
variants, matching the generic add block behavior already reflected in
predicate.jinja and the command’s JSON output support.
In `@data/help/commands/maintenance/chapter/set-source.jinja`:
- Line 8: The usage line for the set-source command is missing the supported
--json flag, so update the set-source usage text in the command template to
include --json alongside the existing options. Make the change in the set-source
help/usage definition so the documented syntax matches the command behavior and
returns JSON is clearly advertised.
In `@src/cli/args.ts`:
- Around line 3034-3056: The create command in args.ts is still silently
accepting unsupported read-only flags like --query and --reverse by omitting
them from the returned args. Add explicit rejection for those flags alongside
the existing rejectArchiveFlag/rejectArchiveBooleanFlag checks in the create
argument parsing path, so invalid invocations fail with a help-linked error
instead of being accepted and dropped.
---
Outside diff comments:
In `@src/cli/archive.ts`:
- Around line 464-501: The temp directory cleanup in createEmptyArchive is only
protected after DirectoryDocument.open, so a failure there can leak the
directory created by createWikiGraphTempDirectory. Move the
DirectoryDocument.open handling inside the existing cleanup try/finally (or add
an outer finally around it) so directoryPath is always removed, and make sure
document.release is still called only when open succeeds.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcaf47f1-9ba4-496a-8e3c-ca7120220bc9
📒 Files selected for processing (17)
data/help/commands/maintenance/chapter.jinjadata/help/commands/maintenance/chapter/add.jinjadata/help/commands/maintenance/chapter/set-source.jinjadata/help/commands/predicate.jinjadata/help/commands/transform.jinjadata/help/topics/format.jinjadata/help/topics/recipe.jinjadata/help/topics/runtime.jinjadata/help/topics/uri.jinjasrc/archive/query/archive-view.tssrc/cli/archive-chapter.tssrc/cli/archive.tssrc/cli/args.tstest/archive/query/archive-view.test.tstest/cli/archive-chapter.test.tstest/cli/archive.test.tstest/cli/args.test.ts
Summary
Verification