Skip to content

fix(server): PR watch wakes the agent when a bot edits its review comment - #15415

Open
Gigioxx wants to merge 1 commit into
pingdotgg:mainfrom
Gigioxx:t3code/issue-15282-fix
Open

Gigioxx wants to merge 1 commit into
pingdotgg:mainfrom
Gigioxx:t3code/issue-15282-fix

Conversation

@Gigioxx

@Gigioxx Gigioxx commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

watch_pull_request only treats a comment as new when its createdAt is past the watch's watermark. Bots like Greptile, CodeRabbit, and Macroscope re-review a push by editing their existing summary comment, which keeps the same id and createdAt, so the re-review never wakes the agent. If CI finishes first, the agent sees no review, ends its turn, and nothing wakes it again.

Fixes #15282.

How I fixed it:

  • PullRequestComment (and thread comments) get an optional editedAt. GitHub fills it from GraphQL lastEditedAt. I used lastEditedAt instead of updatedAt because reactions also move updatedAt, which would cause comment-only wakes for nothing.
  • gh pr view --json comments,reviews does not expose lastEditedAt, so the existing GraphQL review-thread read also selects it for issue comments and reviews and merges it by node id, the same way reactions already arrive.
  • The watcher uses editedAt ?? createdAt as each remark's activity time, keeping the same watermark and same-second id logic. Edits count toward the existing 10 comment-only wake limit, so a bot that keeps rewriting a progress comment cannot loop an agent. Hosts without an edit time keep today's behavior.

Known limits:

  • Edit times cover the first 100 issue comments and reviews, the same ceiling the reaction merge already has.
  • The edit time and the body come from two parallel reads, so a bot edit that lands between them can show the old snippet in the wake. The agent is still woken and reads the PR itself.

Verification:

  • Confirmed on real data: on feat(server): agents can watch a PR and get woken when checks, reviews, or conflicts need them #15057, github-actions, macroscopeapp, and coderabbitai all have comments created around 06:11 to 06:26 and edited hours later (lastEditedAt 09:53 to 10:03), so today none of those re-reviews wake a watching agent.
  • Added focused tests: an edited comment past the watermark wakes the agent, and an old unedited one does not; GitHub JSON decoding reads lastEditedAt for comments, reviews, and thread comments.
  • vp test run src/orchestration-v2/pullRequestWatch.test.ts src/pullRequest/gitHubPullRequestJson.test.ts src/pullRequest/GitHubPullRequestProvider.test.ts in apps/server: 2 failed before the fix, 133 passed after.
  • Server and contracts typecheck and targeted lint pass.
  • Not checked: a live watch end to end against a bot editing a comment.

Reviewed with Codex (GPT-6-Astra, medium). Its two findings are the known limits above.

Created with Claude Opus 5.5 in Claude Code, with implementation by GPT-6-Astra (Codex) through T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 4, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 3129ef2

Macroscope's review found this PR approvable — This is a localized fix to the existing pull-request watch watermark, adding optional edit-time propagation so rewritten bot comments are recognized as new activity. The change is backward-compatible and covered by focused watch, decoding, and provider tests, with no product-default or static-analysis changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (2)
docs/internals/effect-services.md — auto-discovered
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b63e22a1-e9c3-4158-9c97-4c4c1894a423
📥 Commits

Reviewing files that changed from the base of the PR and between c5bc98d and 3129ef2.

📒 Files selected for processing (8)
  • apps/server/src/orchestration-v2/pullRequestWatch.test.ts
  • apps/server/src/orchestration-v2/pullRequestWatch.ts
  • apps/server/src/pullRequest/GitHubPullRequestCli.ts
  • apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts
  • apps/server/src/pullRequest/GitHubPullRequestProvider.ts
  • apps/server/src/pullRequest/gitHubPullRequestJson.test.ts
  • apps/server/src/pullRequest/gitHubPullRequestJson.ts
  • packages/contracts/src/pullRequest.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

GitHub pull-request comments and reviews now carry edit timestamps through decoding and provider activity results. Pull-request watch evaluation uses an edit timestamp, when present, to identify fresh remarks and advance its watermark. Tests cover one-time reporting and wake limits.

Changes

Edited Pull Request Remarks

Layer / File(s) Summary
Decode GitHub edit timestamps
packages/contracts/src/pullRequest.ts, apps/server/src/pullRequest/gitHubPullRequestJson.ts, apps/server/src/pullRequest/gitHubPullRequestJson.test.ts
Comment schemas accept optional edit timestamps. GitHub queries request lastEditedAt, and decoders expose edit times on comments and collect them by ID. Tests cover decoded timestamps and cases without an edit time.
Propagate edit timestamps to activity comments
apps/server/src/pullRequest/GitHubPullRequestCli.ts, apps/server/src/pullRequest/GitHubPullRequestProvider.ts, apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts
The CLI returns edit timestamps by ID. The provider applies them to activity comments, with a fallback for failed thread retrieval. Tests cover the map and returned comment timestamps.
Evaluate edited remarks as fresh activity
apps/server/src/orchestration-v2/pullRequestWatch.ts, apps/server/src/orchestration-v2/pullRequestWatch.test.ts
Watch evaluation uses editedAt when present and otherwise uses createdAt for freshness and the remark watermark. Tests verify that an edit is reported once, retains its ID, and counts toward the wake limit.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: ⚪ Minimal · up to 3129e

This change makes the pull-request watch wake the agent when a bot edits an existing review comment. Focused tests pass, and no merge-blocking risk was found. The disclosed limits are the first-100 cap on edit times and a possible stale snippet if an edit lands between reads.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3129e

Edited review comments can now trigger follow-up work. Self-comment filtering and the limit on consecutive comment-only updates remain in place. No new permission bypass was identified, but racing reads and downstream execution have not been verified end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is to conversations already watching the affected pull request: an externally authored remark edit can produce a queued follow-up message. The same pull request may affect multiple active watchers. The inspected change does not establish additional cross-tenant, credential, or environment authority; downstream execution privileges remain outside the completed trace.

Security Findings and Attack Paths

  • inferred — Someone able to edit an included external remark can now retrigger its delivery after the watermark. This extends event reachability through a pre-existing path that already placed new external comment snippets in follow-up messages; it does not, by itself, demonstrate an authorization bypass or newly granted tool authority.

Trust Boundaries and Controls

  • observed — Timestamp enrichment is provider-owned and keyed to existing GitHub comment IDs. It does not replace comment identity or carry an authority-bearing field. Watch-sync application checks the linked pull-request identity and watch generation under the thread lock, and rejects wakes for archived, settled, or provider-native subagent threads.

Resilience and Maintainability Implications

  • observed — Watch-state mutation and wake-message planning occur in one command. Events, projections, pending effects, and the command receipt are committed transactionally. Receipt replay is thread-bound. These existing mechanisms provide source-level failure containment for edit-triggered wakes; external execution after commit was not verified end to end.
  • observed — Degraded conversation reads do not advance the remark watermark. Closed or merged pull requests terminate watching, exhausted comment-only budgets clear the watch, and generation checks prevent a late read from restoring a stopped or restarted watch.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: the pull-request watch wakes the agent when a bot edits an existing review comment.
Description check ✅ Passed The description covers the problem, implementation, scope, and focused verification. It links issue #15282, but does not state explicit maintainer approval or explain why the change qualifies for the …
Linked Issues check ✅ Passed Issue [#15282] requires the pull-request watch to report an edited review comment as new activity. evaluatePullRequestWatch uses editedAt ?? createdAt for freshness and watermark updates. The GitH…
Out of Scope Changes check ✅ Passed The changes in pullRequestWatch.ts, the GitHub comment and review decoders, the provider, contracts, and their tests support issue [#15282]. The test fixture updates and removal of an unused type im…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: watch_pull_request misses re-reviews from bots that edit their comment in place

1 participant