Repository navigation
Certify complete local reads before Jira export and import - #130
Merged
Merged
Conversation
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (5)
📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. Summary by CodeRabbit
WalkthroughThe change replaces subprocess-based item reads with certified SDK reads. Import and sync paths use existing Jira URLs to skip duplicates, while atomic imports retain a transaction ID for recovery. Tests and documentation cover complete reads, matching, refusal cases, and retries. ChangesCertified local reads and Jira synchronization
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant runImport
participant readPmItems
participant existingJiraUrls
participant importJiraAtomic
runImport->>readPmItems: Read and certify local items
readPmItems-->>runImport: Return complete PmItem list
runImport->>existingJiraUrls: Index Jira URLs and item IDs
existingJiraUrls-->>runImport: Return existing item index
runImport->>importJiraAtomic: Submit pending issues and transaction ID
Merge Risk: ⚪ Minimal · up to Export and import refuse failed complete reads before writing. No merge-blocking issue was established; the change is ready for normal merge checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR makes Jira export and both import paths depend on a certified, in-process, whole-tracker read rather than a bounded CLI JSON decode, adds strict pre-write validation and recovery errors, makes imports skip existing Jira identities across all lifecycle states with resumable atomic semantics, and backs the behavior with real-tracker integration tests and aligned SDK packaging. Sequence diagram for certified Jira exportsequenceDiagram
participant Export as JiraExport
participant Reader as readPmItems
participant SDK as pmCliSDK
participant Certifier as certifyPmItems
participant Jira as JiraAPI
Export->>Reader: readPmItems(pmRoot)
Reader->>SDK: listAllComplete({ includeBody: true }, { pmRoot, cwd, noExtensions: true })
SDK-->>Reader: complete-list result with certificate
Reader->>Certifier: certifyPmItems(candidate)
Certifier->>SDK: certifyCompleteListResult(candidate)
alt certificate and payload fields valid
Certifier-->>Export: certified items
Export->>Jira: create issue payloads
else incomplete or malformed local read
Certifier-->>Export: CommandError with recovery command
end
Sequence diagram for duplicate-safe Jira importsequenceDiagram
participant Import as JiraImportOrSync
participant Reader as readPmItems
participant SDK as pmCliSDK
participant Index as existingJiraUrls
participant Tracker as LocalTracker
Import->>Index: existingJiraUrls(pmRoot)
Index->>Reader: readPmItems(pmRoot)
Reader->>SDK: listAllComplete({ includeBody: true }, { pmRoot, cwd, noExtensions: true })
SDK-->>Reader: certified complete local corpus
Reader-->>Index: all local items
Index-->>Import: Jira browse URL index
Import->>Import: filter pending issues by Jira URL
alt pending issues exist
Import->>Tracker: createPmItem(pmRoot, item)
else all issues already imported
Import-->>Import: report zero newly imported items
end
State diagram for atomic Jira import recoverystateDiagram-v2
[*] --> ReadCertified
ReadCertified --> ExistingIndexed
ExistingIndexed --> PendingPlanned
ExistingIndexed --> CompleteReplay: all Jira URLs match
PendingPlanned --> AtomicCommit
AtomicCommit --> Imported
AtomicCommit --> Interrupted
Interrupted --> AtomicCommit: importJiraAtomic recovery
Imported --> [*]
CompleteReplay --> [*]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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.
Fixes #128
The previous exporter read
pm list --full --include-body --limit 10000and decodeditems,results, or an empty fallback as the whole corpus. That excluded terminal work, capped active rows, and accepted partial or omitted answers. Export now certifies its complete local input before planning or pushing. Both import entrypoints also certify their existing-item index before local writes and skip previously imported issues across every lifecycle state.The issue's October 5 correction is respected: the old bounded reader had only an export caller. Import matching and zero-create re-import behavior are explicit additions required by this task, rather than a claim that the old importer called that reader.
Mechanism and design
listAllComplete({ includeBody: true }, { pmRoot, cwd, noExtensions: true })API. Resolve relative tracker roots once so root and working-directory resolution agree. No local-read subprocess, rendered JSON buffer, or 10,000-row ceiling remains.certifyCompleteListResult. Its certificate establishes complete source scanning, all statuses, full metadata, unique identifiers, no pagination, and no field/output omission or compaction. Additionally require body inclusion and validate every consumed payload/provenance field, because the SDK certificate does not validate those row field types or body inclusion.CommandErrorwith a tracker/SDK recovery hint and the full strict/all-status/unbounded inspection command. A certified empty tracker succeeds; an absent/uninitialized tracker fails.Refusal table
Real regression tests and revert proof
test/complete-local-reads.test.tscreates disposable trackers through the real installed pm CLI and generates real TOON items plus hashed history with public SDK serialization/history/file-writer APIs in process. A sampled history is verified through the CLI. No SDK is mocked, and no real Jira/Linear service or live provider credential is used.The focused suite passes 60 tests, covering 10,003 items (10,001 active plus closed/canceled work), retained final bodies, 51 refusal cases derived from a real SDK envelope, corrupt-tracker refusal before export push/import writes, durable no-write snapshots, terminal matching, both import modes, zero-create repeat imports, URL/body provenance, duplicate local identities, empty/missing trackers, relative roots, and real SDK interruption recovery. Existing exporter runtime tests now populate real trackers; config-driven sync also verifies a zero-create repeat import.
Negative control: restore the baseline CLI reader and original
runImport; expose the old envelope decode expression through the validation test entrypoint while retaining test exports. The unchanged selected regressions exit 1 with 57 failures, including all 51 refusal cases and both terminal-matching modes (3 creates instead of 1). A separate reader-only revert against the final large fixture returns exactly 10,000 instead of 10,003 and exits 1. Restore the implementation: the focused suite and full gates pass.Gates and scope
Exact commit: 3465e21
node --test test/complete-local-reads.test.ts: 60 passed; also passed throughpm test --run --match complete-local-reads --progress.npm run release:check: 254 passed; exact configured 100% lines, branches, and functions.bun run release:check: 254 passed; same unchanged thresholds.npm run changelog:fullran after tracker writes. No version/tag/release/publish action was performed.Coverage's configured executable inventory is
index.tsandsdk-importer.ts; this does not claim operational-script coverage. The Bun release command uses the repository's Node coverage runner and is not independent native-Bun SDK-runtime certification. Test-result tracking remains disabled by existing policy; test results and decisions are recorded in tracker comments.Out-of-scope follow-up pm-jira-d3tp records the existing misleading compensation message on a resumable SDK interruption. Recovery is preserved and tested; that diagnostic remains open. The SDK payload/body certificate gap is handled locally; no upstream issue was filed.
Work item: pm-jira-sdwp. Claim released without closing.
Detailed contract and evidence: docs/complete-local-reads.md.
Summary by Sourcery
Certify complete local tracker data before Jira export or import and make imports idempotent across all item lifecycle states.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores:
Summary by cubic
Fixes #128. The exporter previously read
pm list --full --include-body --limit 10000and decodeditems,results, or an empty fallback as the whole corpus, which dropped terminal work, capped active rows, and accepted partial output. Export and both import entrypoints now certify a complete local read through the public SDKlistAllCompleteAPI before planning or writing anything.CommandErrornaming the full strict all-status inspection command; a certified empty tracker still succeeds.importPmSdkat the complete read; a missing SDK is refused there before any planning or writing.Migration
manifest.jsonminimum move to@unbrained/pm-cli2026.10.4.Written for commit b139f9f. Summary will update on new commits.