Repository navigation
Use the pm SDK for GitHub import lifecycle writes - #180
Merged
Merged
Conversation
![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.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Reviewer's GuideThe PR migrates GitHub import lifecycle finalization from handwritten lock, persistence, rollback, history, and hook handling to the pinned SDK’s Sequence diagram for SDK-backed GitHub import finalizationsequenceDiagram
participant Import as GitHub import
participant Finalize as persistImportFidelity
participant SDK as mutateItem
participant Hooks as Import hooks
participant Store as Item and history storage
Import->>Finalize: persistImportFidelity(pmRoot, id, fidelity)
Finalize->>SDK: mutateItem(pmRoot, id, mutate)
SDK->>SDK: Apply governance and ownership guards
SDK->>Finalize: mutate(document)
Finalize->>Finalize: Normalize lifecycle fields and validate chronology
Finalize-->>SDK: changedFields or no-op
SDK->>Store: Persist item and append history
SDK->>Store: Refresh derived index
SDK->>Hooks: Dispatch import and import:history hooks
SDK-->>Finalize: Complete or refuse before writes
Finalize-->>Import: Finalization result
Flow diagram for transactional import lifecycle mutationflowchart TD
A[GitHub import finalization] --> B[mutateItem boundary]
B --> C{Governance and ownership guards pass?}
C -- No --> D[Refuse before item or history writes]
C -- Yes --> E[Apply normalized lifecycle fields]
E --> F{Chronology valid and fields changed?}
F -- No change --> G[No-op retry]
F -- Changed --> H[Persist item and history atomically]
H --> I[Refresh derived index and dispatch both import hooks]
File-Level Changes
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.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
commented
Oct 25, 2026
unbraind
left a comment
unbraind
left a comment
Owner
Author
There was a problem hiding this comment.
Orchestrator review (built-in Claude review agent, verified by the orchestrator; bots are not waited for since 2026-10-22):
Pre-merge review (codex gpt-6.1-sol read-only review agent; merged by the orchestrator's pipeline after a MERGE verdict and a private-data scan of the added lines).
VERDICT: MERGE
FINDINGS:
- none
SUMMARY: The PR moves import lifecycle persistence into the pm SDK mutation boundary while preserving source dates, hooks, and retry behavior. The tests cover guards, ownership, hook fields, clock changes, and rollback; I reviewed them without executing them.
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
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.
GitHub import lifecycle finalization manually locked items, wrote documents, appended history, rolled back failed appends, and dispatched hooks. It now uses the pinned pm SDK's
mutateItemboundary while preserving exact source lifetimes, release retention, normalization, no-op retries, and both import hooks. The SDK owns the native clock and no-op handling. Hook fields are projected lazily from its live document so they match the actual appended patch even across a clock tick or format enrichment.Tracker: pm-github-d26g, linked to earlier SDK adoption pm-github-gh83. Related lifecycle contract: pm-github-c093.
import-fidelity.ts:445-454: manual item lock and document/history boundarymutateItemowns locks, clock, no-op handling, persistence, rollback, index refresh and hooks.createHistoryEntryonly lazily projects the established JSON-pointer hook fields; local persistence and clock code are deleted.test/sdk-adoption.test.ts; existingtest/import-fidelity.test.ts; differential replaycommitItemMutationsfor ordinary conditional fieldsmutateItemboundary. No missing SDK API blocker.docs/sdk-adoption-1025.mdcloseout.ts:357-368chronology/attributionDifferential proof: built origin/main
1cb49ac06bfee8d1193137367f86a36798a1040eversus this implementation on disposable, non-git copies of complete public pm-github and pm-brief trackers: 14/14 scenarios pass. Normal cases compare exact item/history bytes and import hook payloads with a frozen clock and tracker-relative paths. Retries preserve bytes and emit no import hooks. Source snapshot digests remain unchanged. The replay harness and exact snapshot commits are documented indocs/sdk-adoption-1025.md.Intentional SDK differences: transactional guard/strict ownership refusals use conflict exit code 4; missing-item diagnostics use not-found exit code 3 instead of generic exit 1; derived-index maintenance adds runtime lock observations; legacy document updates use SDK-native TOON storage. README and the evidence document describe these contracts. Public differential inputs use native TOON.
Revert proof: restore only
persistImportFidelityand its imports to origin/main, then runnode --test test/sdk-adoption.test.ts: 2 pass / 2 fail because guard and ownership refusals disappear; hook/retry and clock-boundary compatibility pass. Reverting only the lazy field projection to the eager SDK adapter yields 3 pass / 1 fail, omittingupdated_atfrom hook fields despite its presence in the history patch. The corrected implementation passes 4/4 under Node and native Bun, with real trackers and no SDK mocks.Validation:
npm run typecheck,npm run build:test,npm run lint,npm run docstring,npm run duplication, linkedpm test --run --progress, changelog generation/check, and isolated strict tracker health pass. Local Node and native Bun regressions each pass 4/4; production differential replay passes 14/14.The final-head CI matrix passed 4/4 jobs for
c4604a6d1a6932e68d40bc50c56de53a5c11ac99:Every release gate retains exact 100/100/100/100 across 19 authored modules at unchanged thresholds. CodeQL and Semgrep also passed. Independent review remains pending; unavailable/skipped/neutral automated responses are not approval.
Production diff: 24 removed / 17 added lines ignoring indentation (34 removed / 27 added including indentation), net seven fewer lines. No dependency bump.