Skip to content

Fix triggered monitor PR sync#139

Merged
alvarosanchez merged 2 commits into
mainfrom
codex/fix-triggered-monitor-sync
May 25, 2026
Merged

Fix triggered monitor PR sync#139
alvarosanchez merged 2 commits into
mainfrom
codex/fix-triggered-monitor-sync

Conversation

@alvarosanchez
Copy link
Copy Markdown
Owner

Summary

  • Treat only scheduled Paperclip issue monitors as sync-owned waits.
  • Allow fired triggered monitors to reconcile against live linked PR state instead of freezing terminal Paperclip issues.
  • Add regression coverage for a done issue with an open direct PR link and a triggered monitor, and keep scheduled-monitor abstention coverage.

Why

PR #130 correctly stopped GitHub Sync from competing with monitors that are still waiting to fire, but it treated triggered monitor state as still active. In real Paperclip monitor lifecycle data, a monitor can remain visible as triggered after it fires. That made manual or scheduled sync skip status repair even when the linked GitHub pull request was still open.

Validation

  • pnpm typecheck
  • pnpm test
  • pnpm build
  • git diff --check
  • pnpm test:e2e against disposable paperclipai@2026.517.0

Model Used

  • Model ID/version: GPT-5 Codex via the Codex desktop app
  • Context window: not exposed by the runtime in this session
  • Capabilities used: code editing, shell commands, GitHub CLI, local Paperclip verification harness

Copilot AI review requested due to automatic review settings May 25, 2026 11:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts GitHub Sync’s interaction with Paperclip issue monitors so that only scheduled monitors block sync-driven state reconciliation, while monitors that have already fired (status triggered) no longer prevent sync from re-evaluating the live linked PR state.

Changes:

  • Update monitor abstention logic to treat only scheduled issue monitors as “active” for sync-skipping decisions.
  • Add a regression test covering a done Paperclip issue with a direct PR link and a triggered monitor, ensuring sync can reopen it when the PR is still open/failing.
  • Update SPEC/README wording to reflect the scheduled-vs-triggered monitor contract.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/worker.ts Changes the “active monitor” predicate to only block sync when monitor status is scheduled.
tests/plugin.spec.ts Adds regression coverage for triggered-monitor reconciliation and keeps scheduled-monitor abstention coverage.
SPEC.md Updates the spec to define abstention only for scheduled monitors and allow reconciliation for triggered.
README.md Updates documentation to describe scheduled monitor ownership and post-fire reconciliation behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/worker.ts Outdated
@alvarosanchez alvarosanchez merged commit e606320 into main May 25, 2026
1 check passed
@alvarosanchez alvarosanchez deleted the codex/fix-triggered-monitor-sync branch May 25, 2026 12:03
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.

2 participants