Skip to content

fix(gitlab): persist unposted inline comments + split browser-safe types#719

Merged
backnotprop merged 1 commit into
mainfrom
fix/680
May 13, 2026
Merged

fix(gitlab): persist unposted inline comments + split browser-safe types#719
backnotprop merged 1 commit into
mainfrom
fix/680

Conversation

@backnotprop
Copy link
Copy Markdown
Owner

Summary

Closes #680.

  • submitGlMRReview (packages/shared/pr-gitlab.ts): when one or more inline discussion POSTs fail (e.g. transient i/o timeout against GitLab), the failed comment bodies are now persisted to ~/.plannotator/failed-comments/{host}-{project}-mr{iid}-{ts}.json so they can be recovered. The error message points the user at the file path.
  • The original throw-vs-warn split is preserved deliberately and now commented in the source:
    • All inline comments failed → throw. Nothing was posted, so the UI retries from a clean state.
    • Partial failure → warn only. The MR note and the discussions that did succeed are already on GitLab; throwing would have the client resubmit the whole review and create duplicates.

Why the diff is bigger than #680 alone

The persistence call needs fs/os/path. The review-editor browser bundle was pulling pr-gitlab.ts in as dead code through pr-provider.ts's static imports, which made the file silently constrained to "no Node-only imports." Adding fs broke bun run --cwd apps/review build.

So I split pr-provider.ts along its existing seam:

  • pr-types.ts (new) — types, label helpers (getPlatformLabel, getMRLabel, ...), parsePRUrl. No imports of pr-github / pr-gitlab. Browser-safe by construction.
  • pr-provider.ts — server-only dispatch (submitPRReview, fetchPR, ...). Re-exports pr-types so existing server-side imports keep working unchanged.
  • Browser code in packages/review-editor/* now imports from pr-types (~14 files, all type-only or pure-helper imports).

Test plan

  • bun test packages/shared/ — 272 pass, 0 fail
  • tsc --noEmit on packages/shared, packages/ai, packages/server, packages/ui, apps/pi-extension — all clean
  • bun run --cwd apps/review build — review browser bundle no longer fails on __vite-browser-external
  • bun run build:hook — full hook build green
  • bun build apps/hook/server/index.ts --compile — single-file binary builds (107 MB output)
  • vendor.sh updated to vendor pr-types.ts into apps/pi-extension/generated/; Pi typecheck clean
  • Manual: trigger a GitLab MR review against a flaky network and confirm ~/.plannotator/failed-comments/*.json contains the unposted bodies, while already-posted comments are not duplicated on retry

…m browser-safe pr-types

Closes #680. Two changes that landed together because the persistence fix
exposed a hidden architectural constraint.

1. GitLab inline comments: when one or more discussion POSTs failed
   (e.g. transient `i/o timeout`), the failed comment bodies were lost.
   Now `submitGlMRReview` writes them to
   `~/.plannotator/failed-comments/{host}-{project}-mr{iid}-{ts}.json`
   in both the all-fail and partial-fail branches. The throw-vs-warn
   split is preserved deliberately: all-fail throws so the UI retries
   from a clean state, partial-fail warns so the UI doesn't resubmit
   already-posted content.

2. Split `packages/shared/pr-provider.ts` into `pr-types.ts`
   (browser-safe types + pure label/URL helpers) and `pr-provider.ts`
   (server-only dispatch that imports pr-github / pr-gitlab). The
   review-editor browser bundle previously dragged pr-gitlab.ts in as
   dead code via static imports, which silently constrained the file
   to never use Node built-ins. Adding `fs`/`os`/`path` for (1) broke
   the review build until we routed browser imports to pr-types and
   left server callers on the now server-only pr-provider facade.

Server-only `pr-provider.ts` re-exports `pr-types` so existing
server-side imports keep working unchanged.
@backnotprop backnotprop merged commit 3fb0b9c into main May 13, 2026
10 checks passed
@backnotprop backnotprop deleted the fix/680 branch May 13, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inline comments lost on GitLab API timeout — no retry or local fallback

1 participant