Skip to content

fix: extend realtime sync mutation coverage#724

Open
jaeyunha wants to merge 3 commits into
stagingfrom
issue-560-parity-complete-realtime-sync-cove
Open

fix: extend realtime sync mutation coverage#724
jaeyunha wants to merge 3 commits into
stagingfrom
issue-560-parity-complete-realtime-sync-cove

Conversation

@jaeyunha

Copy link
Copy Markdown
Member

Summary

  • Subscribe to Redis before replaying sync operations to close the connect/replay race.
  • Emit and publish transactional operations for team cycles, bulk labels, and project label mutations.
  • Expand web sync route refresh coverage for label, project label, cycle, team, view, and custom emoji operations.

Verification

  • pnpm --filter @namuh-eng/expn-sdk test
  • pnpm exec biome check apps/web/src/app/(app)/sync-subscription.tsx
  • make check (blocked by existing web typecheck syntax errors in integrations/page.tsx, issue-detail-view.tsx, and integrations-view.test.tsx)
  • make test (blocked: go binary missing in this environment)

Refs #560

jaeyunha and others added 3 commits June 16, 2026 09:26
Union-merged figma.SyncSources calls from staging into issues/handler.go
Create and Update handlers, preserving the op return value from
insertOperation needed by syncapi.PublishOperations. Regenerated
packages/sdk/src/generated.ts from the auto-merged openapi.yaml.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jaeyunha

Copy link
Copy Markdown
Member Author

Controller disposition for current head 748842c: validation-blocked / rebase required; do not merge as-is.

Evidence:

  • PR fix: extend realtime sync mutation coverage #724 targets staging but does not contain current origin/staging; GitHub reports mergeable=CONFLICTING.
  • Local merge simulation reproduces conflicts across realtime sync mutation files plus OpenAPI/SDK and web subscription surfaces.
  • Worker lane is still dirty (.env.1password typechange, untracked package-lock.json) and the final artifact path is missing, so the PR body is the only validation record.
  • PR body claims SDK tests + focused Biome passed, but broad make check/make test were blocked; no GitHub checks reported.

Required before merge: port only the realtime mutation coverage delta onto current origin/staging, remove dirty workspace residue, regenerate OpenAPI/SDK if the API contract changes, and rerun focused sync Go + SDK/web gates plus broader repo gates as far as the environment allows.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 748842ca3b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
problem.JSON(w, 200, bulkReadResponse{Success: true, UpdatedCount: cmd.RowsAffected(), UnreadCount: count})
response := bulkReadResponse{Success: true, UpdatedCount: cmd.RowsAffected(), UnreadCount: count}
op, err := syncapi.InsertOperation(r.Context(), tx, p.WorkspaceID, "notification", p.UserID, "bulk_read", response, p.UserID)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep notification sync events user-scoped

When a user marks notifications read, this inserts a notification operation under p.WorkspaceID and later publishes it on the workspace channel; since SubscribeOperations subscribes by workspace, every member connected to that workspace receives this user's notification action and unreadCount payload. Notification rows are otherwise filtered by user_id, so these sync operations should be routed/filterable per user (or not published to the shared workspace stream) to avoid leaking per-user inbox state.

Useful? React with 👍 / 👎.

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.

1 participant