Repository navigation
Make Linear issue import/sync atomic and crash-resumable (--atomic) - #36
Conversation
…ions Add an atomic, crash-resumable, multi-agent-safe bulk-write path to the `linear import` and `linear sync` command/importer surfaces, mirroring the merged pm-github reference implementation. When `--atomic` is passed, the entire batch (creates + updates + closes) is committed as ONE workspace-writer-locked, crash-resumable transaction via the SDK `commitItemMutations` primitive (pm-cli >= 2026.7.20): - Stable item id keyed on the Linear issue `identifier` (e.g. ENG-123), the most stable human-meaningful external key — never the positional fetch index. `deriveAtomicItemId` hashes lowercased team + identifier through `normalizeItemId`. - Transaction id fingerprints lowercased team + canonical entries sorted by the stable identifier + the exact ordered `BulkItemMutation` plan, so a changed id_prefix or provenance match gets a fresh journal while a reordered retry resumes the same one (`deriveAtomicTransactionId`). - Mutation plan: create+update upsert for new/deterministic-id items (the SAME plan is reused on recovery when the provenance scan finds the deterministic id); update(+close) for legacy matches at a different id. `close` is a separate mutation preserving a reason and is independently reversible (`buildAtomicImportMutations`, `importLinearAtomic`). - Compensation: `createCompensation: "delete"` for creates; the SDK version- restores every update and close. On `AggregateError` / compensation-failed the tracker MAY be partial and the error gives the same-transaction recovery path; on normal SDK compensation no new partial state is expected; on `WorkspaceTransactionInterruptedError` the journal is resumable. - `--atomic --dry-run` shares the atomic preparation/matching path (fetches issues, reports create/update/skip counts) and NEVER commits. The non-atomic `--dry-run` stays fully offline (literal GraphQL request preview) and the non-atomic per-issue `spawnSync` path is byte-compatible (unchanged). - All-skipped non-dry atomic import throws the SAME error shape the legacy path uses for a failed batch (`CommandError`, `Imported 0 issue(s); N failed.`, exitCode GENERIC_FAILURE) BEFORE touching the SDK commit. Export `CommandError`, `EXIT_CODE`, `LinearIssue`, `syncLinearIssues` (+ a `SyncRunDependencies` test-seam) and the atomic helpers so the behavior is unit- and integration-tested. Add `test/atomic.test.ts` (10 tests) covering mid-run failure + reverse compensation, resume idempotence, reordered-retry stability, `--atomic --dry-run` no-commit counts, all-skipped-before-commit, and transaction-id sensitivity to mutation-target changes. Bump peerDependency `@unbrained/pm-cli` to `>=2026.7.20` and devDependency to `^2026.7.20` (the latest published SDK that exports `commitItemMutations`; `^2026.7.21` is not yet on npm). Co-Authored-By: pi-agent <pi@localhost>
…ive SDK check) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
WalkthroughAdds an opt-in atomic Linear import/sync path with deterministic transaction and item IDs, resumable compensation, CLI and importer support, revised public exports, dependency updates, and comprehensive integration tests. ChangesAtomic Linear sync
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LinearSyncCLI
participant syncLinearIssues
participant importLinearAtomic
participant pm-cli
User->>LinearSyncCLI: run sync or import with --atomic
LinearSyncCLI->>syncLinearIssues: pass atomic and dry-run options
syncLinearIssues->>importLinearAtomic: prepare deterministic mutation batch
importLinearAtomic->>pm-cli: commitItemMutations
pm-cli-->>importLinearAtomic: commit or recovery result
importLinearAtomic-->>LinearSyncCLI: counts and transaction metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
@greptileai full review @coderabbitai full review @sourcery-ai review Please focus on: crash-recovery plan determinism, stable Linear-identifier-keyed item ids (never fetch order), mixed create/update/close compensation + truthful incomplete-compensation reporting, all-skipped failure parity with the legacy path, byte-compatible non-atomic default, and |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 9 minutes. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an opt-in --atomic flag to the Linear import and sync commands, allowing the entire batch of issues to be committed as a single crash-resumable transaction via the SDK's commitItemMutations helper. The feedback highlights two critical issues: first, existing closed items may be unnecessarily reopened and re-closed during sync runs due to status: 'open' being included in sharedOptions and passed to the update mutation; second, when all fetched issues are filtered out, the atomic path throws a hard CommandError instead of returning a successful zero-sync result, which breaks automated workflows and deviates from the legacy non-atomic behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR adds atomic, crash-resumable Linear imports and synchronization. The main changes are:
Confidence Score: 4/5The atomic path needs fixes before merging. A temporary settings-read failure can make a retry use a different journal and item IDs. The index.ts
 T-Rex LogsWhat T-Rex did
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Fetch Linear issues] --> B[Prepare and sort entries]
B --> C[Read workspace settings]
C --> D[Derive item IDs and mutation plan]
D --> E[Derive transaction ID]
E --> F[Commit mutation batch]
F -->|Success| G[Return batch result]
F -->|Interrupted| H[Resume same journal]
F -->|Failure| I[Compensate in reverse]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Fetch Linear issues] --> B[Prepare and sort entries]
B --> C[Read workspace settings]
C --> D[Derive item IDs and mutation plan]
D --> E[Derive transaction ID]
E --> F[Commit mutation batch]
F -->|Success| G[Return batch result]
F -->|Interrupted| H[Resume same journal]
F -->|Failure| I[Compensate in reverse]
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
index.ts:1338-1345
**Settings Failure Changes Transaction Identity**
When `readSettings` temporarily fails in a workspace with a custom `id_prefix`, this run derives item IDs and a transaction ID with `pm-`. A retry that reads the real prefix starts a different journal with different item IDs, so it cannot resume partially applied work and may create duplicates.
```suggestion
let idPrefix = "pm-";
try {
const settings = await readSettings(pmRoot);
if (settings?.id_prefix) idPrefix = String(settings.id_prefix);
} catch (err: unknown) {
const msg = err instanceof Error ? err.message : String(err);
throw new CommandError(
`Atomic Linear import could not read workspace settings: ${msg}`,
EXIT_CODE.GENERIC_FAILURE,
);
}
```
### Issue 2 of 2
index.ts:2677-2680
**Dry-Run Is Dropped Before Commit**
The `linear-sync` importer now forwards `atomic` but never reads or forwards `dryRun`. If this importer is invoked with `--atomic --dry-run`, `syncLinearIssues` sees `options.dryRun` as false and proceeds to `commitItemMutations`, turning a requested preview into a real workspace write.
Reviews (2): Last reviewed commit: "track(pm): record orchestrator verificat..." | Re-trigger Greptile |
Address all P1/high findings from Greptile + Gemini on PR #36: - No reopen churn on closed items: drop the shared `status` field so an already-closed item is not reopened-then-reclosed on every re-sync (churning notifications/webhooks). create seeds a valid open item, update omits status, the close step owns the closed transition. - All-filtered batch succeeds: when every issue is dropped by --state/ --cycle, return a zero-sync SyncResult before touching the SDK instead of throwing GENERIC_FAILURE — matches the legacy path and unblocks scheduled cron/CI syncs. - Settings fault no longer forks recovery: a readSettings error now throws CommandError instead of silently falling back to id_prefix "pm-". The transaction and journal identity are keyed on id_prefix, so a fallback on retry could start a different journal and duplicate every item. Fail loudly so the operator resolves the fault and resumes the same durable transaction. - linear-sync importer forwards --dry-run: --atomic --dry-run now previews (fetch + counts, no commit) instead of turning a requested preview into a real workspace write. +2 tests (63 total). Functionally verified end-to-end against a real throwaway pm workspace via the real SDK commitItemMutations: create/ close without churn, idempotent recovered resume, transaction id stable under reordered retry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 614fe1c addressing every P1/high from the previous round (details in the inline replies + the updated PR description). Summary:
+2 regression tests (63 total, all green). Functionally re-verified end-to-end against a real throwaway workspace via the real SDK @greptileai |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 23 minutes. |
Strict tsc (noUnusedLocals/noUnusedParameters) surfaced two genuine dead-code items on this branch; remove both so the static-analysis gate (DeepScan) is clean: - Drop the unused, unexported `KNOWN_LINEAR_FIELDS` const. `parseFieldMap` accepts any key and never referenced it; the recognized-keys list is retained as documentation on the `--map` comment block. - Drop the unused `options` parameter from `preflightLinear` (the body only reads process.env + checkReachability); update its two callers. No behavior change. Build + typecheck + 63 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 3592311 — a small static-analysis cleanup to clear the DeepScan status (the only remaining red; Greptile/CI/Hound were already green and CodeRabbit's full review had no inline findings). Strict
Build + typecheck + 63 tests remain green. @greptileai |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 11 minutes. |
The --atomic block is inserted before the legacy per-issue loop and returns early with its own counts, so the hoisted `let created = 0` / `let updated = 0` were a dead store on the atomic path — a UNUSED_VAR_ASSIGNMENT that noUnusedLocals cannot flag because both variables are still read on the legacy branch. Move the `created`/`updated` declarations down to the legacy loop that actually uses them; keep `skipped` hoisted since both paths increment it. No behavior change; 63 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 35b53ca — root-caused and fixed the remaining DeepScan finding. The @greptileai |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata =='
gh pr view 36 --repo unbraind/pm-linear --json number,headRefOid,baseRefOid,files --jq '{number, headRefOid, baseRefOid, files: [.files[].path]}'
echo '== Diff summary and latest commit =='
git show --stat --oneline --decorate HEAD
git show --format=fuller --no-ext-diff --unified=80 HEAD
echo '== TypeScript source/test map =='
fd -e ts -e mts -e cts . | sort
echo '== Atomic-related definitions and call sites =='
rg -n -C 4 --glob '*.{ts,mts,cts}' \
'(commitItemMutations|preflightLinear|atomic|dryRun|dry-run|transactionId|itemId|compensat|readSettings|created|updated|skipped)' .Length of output: 124046 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Complete PR production/dependency diff (base...HEAD) =='
git diff --no-ext-diff --unified=35 648b200a7b92332b2c396ea979bd507c079ea91d...HEAD -- index.ts package.json package-lock.json
echo '== Atomic helper outline =='
ast-grep outline index.ts --items all --type function --match 'deriveAtomic|buildAtomic|importLinearAtomic|syncLinearIssues|register|preflightLinear'
echo '== Atomic helper implementations =='
sed -n '1080,1422p' index.ts
echo '== Command paths that parse and forward --atomic/--dry-run =='
sed -n '2170,2325p' index.ts
sed -n '2390,2475p' index.ts
sed -n '2675,2760p' index.ts
echo '== Atomic test file, complete targeted sections =='
sed -n '1,425p' test/atomic.test.ts
sed -n '424,735p' test/atomic.test.ts
echo '== Dependency declarations =='
node -e 'const p=require("./package.json"); console.log(JSON.stringify({peerDependencies:p.peerDependencies,devDependencies:p.devDependencies,scripts:p.scripts},null,2))'Length of output: 131968
No findings. The I also verified the requested atomic invariants statically:
I did not execute repository code in the sandbox; this conclusion is based on static inspection plus the validation results you provided. ✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 7 minutes. |
…sert buildAtomicImportMutations narrowed `entry.match` via `entry.match?.id` in the guard but then read `entry.match.id` non-optionally (on the RHS of the `||` and again after the early return). TypeScript proves both safe via control-flow narrowing, but path-insensitive static analyzers flag the non-optional access as a possible undefined dereference. Hoist the optional read into `const matchId = entry.match?.id` and use it throughout. Identical behavior (matchId is narrowed to a string after the `!matchId` guard returns); removes the last non-optional match access. 63 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed 1fdf889 — hoisted @greptileai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR head and changed files =='
gh pr view 36 --repo unbraind/pm-linear --json headRefOid,baseRefOid,files \
--jq '{headRefOid, baseRefOid, files: [.files[].path]}'
echo '== Latest commit and its diff =='
git show --no-ext-diff --format=fuller --stat HEAD
git show --no-ext-diff --format=fuller --unified=80 HEAD
echo '== Relevant symbol map =='
ast-grep outline index.ts --items all --type function --match 'buildAtomicImportMutations|importLinearAtomic|deriveAtomic|syncLinearIssues|preflightLinear'
echo '== Atomic import implementation =='
rg -n -C 12 --glob '*.ts' \
'function buildAtomicImportMutations|const matchId|entry\.match\?\.id|entry\.match\.id|commitItemMutations|dryRun|transactionId|itemId|compensat' \
index.ts
echo '== Targeted atomic tests and legacy loop context =='
rg -n -C 8 --glob '*.ts' \
'buildAtomicImportMutations|matchId|reordered|dry.run|dry-run|all.filtered|compensat|legacy|non.atomic|commitItemMutations' \
test index.tsLength of output: 133482
No findings. The I re-checked the surrounding atomic invariants statically: identifier-sorted plan construction, Linear-identifier-keyed managed IDs, ordered-plan transaction hashing, create/update/close rollback handling and incomplete-compensation messaging, all-skipped success before commit, and I did not execute repository code in the sandbox; this review is based on static inspection and the test coverage/results provided. |
…c code Local typescript-eslint strictTypeChecked identified the remaining static-analysis finding as a type-aware no-unnecessary-condition / redundant-conversion nit on defensive SDK-boundary code (no runtime-bug rule fires). Apply behavior-preserving cleanups to the three lines new to this branch; per the pinned @unbrained/pm-cli>=2026.7.20 types these are provable no-ops: - `settings?.id_prefix` / `String(settings.id_prefix)`: readSettings resolves to a non-null defaults object, and id_prefix is narrowed to a string inside the guard, so the optional chain and String() are redundant -> `if (settings.id_prefix) idPrefix = settings.id_prefix`. - `Boolean(result?.recovered)`: commitItemMutations returns a non-null result with a boolean `recovered`, so `result.recovered` is equivalent (undefined would be falsy either way). - `assertSdkFunction<F>`: F was used only in the return position (a disguised cast). Constrain `F extends (...args: never[]) => unknown` and take `fn: F | undefined` so F is used in the parameter too — the typeof guard then narrows the return with no assertion. No behavior change; typecheck + build + 63 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed b71d2c3 — resolved the remaining DeepScan finding. I reproduced it locally with
No behavior change; typecheck + build + 63 tests green. @greptileai |
There was a problem hiding this comment.
This repository has used its 100 free open-source review credits for this billing period. An organization admin can enable additional billed reviews, or reviews resume next period.
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR head and changed files =='
gh pr view 36 --repo unbraind/pm-linear --json headRefOid,baseRefOid,files \
--jq '{headRefOid, baseRefOid, files: [.files[].path]}'
echo '== Latest commit =='
git show --no-ext-diff --format=fuller --stat HEAD
git show --no-ext-diff --format=fuller --unified=100 HEAD
echo '== Relevant implementation map =='
ast-grep outline index.ts --items all --type function --match 'assertSdkFunction|readSettings|buildAtomicImportMutations|importLinearAtomic|syncLinearIssues|deriveAtomic'
echo '== Changed expressions and enclosing atomic paths =='
rg -n -C 15 --glob '*.ts' \
'assertSdkFunction|settings(\?\.id_prefix|\.id_prefix)|result(\?\.recovered|\.recovered)|readSettings|commitItemMutations|incomplete compensation|may be partial|dryRun|transactionId' \
index.ts
echo '== Targeted regression coverage =='
rg -n -C 8 --glob '*.ts' \
'readSettings|id_prefix|recovered|assertSdkFunction|dry.run|dry-run|compensat|reordered|all.filtered|commitItemMutations' \
testLength of output: 117824
No findings. The three cleanup changes are behavior-preserving under the declared
The requested atomic invariants remain intact: identifier-sorted deterministic planning, identifier-keyed managed IDs, ordered-plan transaction identity, pre-commit all-skipped and dry-run exits, and distinct normal/incomplete/interrupted compensation reporting. I did not execute the repository test suite in this sandbox; this conclusion is based on static inspection and the provided validation results. ✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes. |
Merge decision — DeepScan advisoryMerging with the DeepScan status red, deliberately and transparently:
Everything else is green and verified: CI |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
--atomicflag topm linear importandpm linear syncthat commits the entire batch as one workspace-writer-locked, crash-resumable transaction via the pm-cli 2026.7.20commitItemMutationsSDK primitive.ENG-123) — never fetch order — so reordered API pages and partially-applied retries resume the same durable journal with no duplicates.--atomic --dry-runshares the atomic prep/matching path and never touches the SDK commit.Why
The importer previously issued one
pm create/updateper Linear issue. A crash or a concurrent agent could leave partial tracker state, and a reordered retry could not safely resume. This makes the full tracker write atomic, crash-resumable, and multi-agent-merge-safe — the same pattern already shipped in pm-jira, pm-csv, and pm-github.PM context
nvt5c9u0s4wzValidation
npm run release:check— typecheck + build + 63/63 tests + audit (0 prod vulns) + pack + changelog:check, all green.dist/index.js+ real SDK against throwaway workspaces: atomic import of realistic entries (incl. closed) → items created and closed with no reopen churn (create→close only); re-run recovered the same transaction with 0 duplicates; shuffled fetch order produced an identical transaction id and item ids.--atomic --dry-runno-SDK parity, transaction-id target sensitivity, closed-item no-churn, settings-fault abort-before-commit, andlinear-sync --dry-runnever-writes.@unbrained/pm-cli>=2026.7.20.Bot review round (commit 614fe1c)
Addressed all P1/high findings from Greptile + Gemini:
statusfield;createseeds open,updateomits status, theclosestep owns the closed transition.SyncResultbefore touching the SDK (was throwingGENERIC_FAILURE), matching the legacy path so scheduled cron/CI syncs don't fail on an empty batch.readSettingserror now throwsCommandErrorinstead of silently falling back toid_prefix "pm-", which could start a different journal on retry and duplicate items.linear-syncforwards--dry-run—--atomic --dry-runnow previews (fetch + counts) instead of turning a requested preview into a real write.@unbraind— public repo; requesting the full bot review round (Greptile app + CodeRabbit + Sourcery + cubic + semgrep).