[1984 by GPT-5.6 Sol] Deduplicate Inbox notifications and automatically close stale PR/system items - #1986
Open
propr-dev[bot] wants to merge 4 commits into
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…tomatically Implemented by ProPR AI using gpt-5.6-sol model. Implementation completed successfully.
Contributor
Author
|
/ultrafix |
Contributor
Author
|
🔄 Ultrafix loop started (goal: 8/10, max cycles: 10) First action:
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 2a5c10e Summary of ChangesImplemented the PR #1986 follow-up fix.
Validation passed:
No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented the PR #1986 follow-up fix. - Extracted projection test setup/query helpers into [notificationProjectionTestHarness.ts](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-20-35/packages/api/test/notificationProjectionTestHarness.ts). - Reduced [notificationProjectionService.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-20-35/packages/api/test/notificationProjectionService.test.ts) below the 400-line ESLint limit. - Compacted notification exports in [index.ts](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-20-35/packages/core/src/index.ts:412). Validation passed: - API-wide lint - Core index lint - API build - Focused projection suite: 11/11 - Full notification regression suite: all server and UI tests - `git diff --check` No commit was created. PR: #1986 Comment by: @github-actions[bot] (ID: 5464949801) Model: gpt-5.6-sol
Contributor
Author
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR takes a sound audit-event/receipt-state approach, but it is not ready to merge because concurrent projections can suppress the current system-failure card or recreate PR notifications after merge. ✅ Audit records remain immutable — Lifecycle operations update receipt dismissal state while retaining notification event rows. ✅ Core sequential behavior is covered — Tests exercise PR-card replacement, cross-instance system deduplication, recovery, and stale snapshot handling; all current checks pass. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Stale projections can dismiss current failures
F2: 🔴 Merge dismissal races notification creation
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe implementation has good structure, focused coverage, and passing checks, but both remaining races affect the central lifecycle guarantees, including the explicit multi-instance requirement. Score: 5/10 🤖 Review Details
🤖 Review by ProPR |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 9eef724
Summary of ChangesImplemented F1 and F2 without committing.
Verification passed:
🤖 Implementation Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented F1 and F2 without committing. - System-failure transitions now atomically update durable state, dismiss only the specifically superseded event, and create the current event. Added a forced-interleaving regression. - PR merges now atomically persist an authoritative merged marker and dismiss receipts. Task, review, stalled-task, and PR-attention projections check that marker before creation. - Added the new PR lifecycle migration in [20260829010000_add_notification_pull_request_state.js](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-45-58/packages/core/src/db/migrations/20260829010000_add_notification_pull_request_state.js). - Updated [notificationService.ts](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-45-58/packages/core/src/services/notificationService.ts), [notificationProjectionService.ts](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-45-58/packages/api/services/notificationProjectionService.ts), and [planIssueTracking.ts](/tmp/git-processor/worktrees/integry/propr/pr-1986-followup-2026-08-29T21-45-58/packages/core/src/webhook/planIssueTracking.ts). Verification passed: - API and core lint - API and core builds - Focused lifecycle race tests - Full notification-service suite: 28/28 - Server notification suite: all 9 files - Migration compatibility: 32/32 - `git diff --check` PR: #1986 Comment by: @propr-ultrafix (ID: 0) Model: gpt-5.6-sol
Contributor
Author
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR adds durable PR-merge and system-health lifecycle state while preserving immutable audit events. The approach is solid overall, and all 11 authoritative checks pass, but two transaction/bootstrap gaps need correction before merge. ✅ Merge-aware projection — PR-related task, review, and attention events are suppressed after the durable merged marker is recorded. ✅ Immutable audit history — Lifecycle cleanup dismisses user receipts without deleting notification events. ✅ Serialized system transitions — Once state is initialized, failure replacement, recovery dismissal, and new-event creation occur in one transaction. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F3: 🔴 PR deduplication is not atomic
F4: 🔴 Existing system cards survive bootstrap
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Index lifecycle target lookupsThe new merge and supersession queries repeatedly filter the immutable event table through ScoreThe implementation has strong coverage and green checks, but the non-atomic PR replacement and missing upgrade bootstrap can both leave duplicate or stale active cards—the behavior this PR is intended to eliminate. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit bbb37f0
Summary of ChangesImplemented F3 and F4 only.
Verification passed:
Changes are uncommitted as requested. 🤖 Implementation Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented F3 and F4 only. - PR-attention creation, recipient assignment, merge check, and supersession now share one transaction. - First system-health snapshots reconcile pre-migration cards while preserving audit events. - Added rollback and upgrade-state regression tests. Verification passed: - Core/API typechecks - Core/API lint - 42 focused notification tests Changes are uncommitted as requested. PR: #1986 Comment by: @propr-ultrafix (ID: 0) Model: gpt-5.6-sol
Contributor
Author
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR is ready to merge. It adds durable PR and system-health lifecycle state, atomically supersedes stale receipts while retaining audit events, and protects against delayed projections recreating merged-PR notifications. ✅ Atomic lifecycle transitions — Event creation, merge-state checks, receipt dismissal, and system-health transitions share database transactions. ✅ Audit history preserved — Supersession and recovery update Inbox receipts without deleting immutable notification events. ✅ Strong race coverage — Tests exercise repeated PR attention, delayed post-merge projection, cross-instance system transitions, rollback behavior, and legacy system cards. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Reconcile historical PR cardsA future maintenance task could deduplicate PR-attention receipts that existed before this migration and optionally reconcile cards for PRs merged before deployment. The new creation and webhook paths enforce the invariant prospectively, but historical PR state cannot be inferred locally without a GitHub reconciliation pass. S2: 🟢 Add webhook integration coverageAn integration test around ScoreThe implementation satisfies the stated lifecycle requirements, includes meaningful concurrency and rollback tests, and all authoritative head checks pass. The remaining opportunities concern historical-data reconciliation and extra integration coverage. Score: 9/10 🤖 Review Details
🤖 Review by ProPR |
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.
AI Implementation Summary
Closes #1984
Branch:
1984/gpt-5.6-sol-deduplicate-inbox-notific-20260829-2104-y8tCommits: ✅ Changes committed (c6f6b45)
AI Processing Completed
Execution Details:
Summary:
Implemented issue #1984.
Key changes:
Validation completed:
Detailed Logs:
01a04f57-5263-7152-bec5-c8c72cab7ce3Log files stored at:
/tmp/claude-logs/issue-1984-2026-08-29T21-17-16-974Z-conversation.json/tmp/claude-logs/issue-1984-2026-08-29T21-17-16-974Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #1984.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!