Skip to content

fix(server): retain consumed immutable check failures - #35394

Open
osnoser1 wants to merge 3 commits into
pingdotgg:mainfrom
osnoser1:fix/consumed-check-failure-notices
Open

osnoser1 wants to merge 3 commits into
pingdotgg:mainfrom
osnoser1:fix/consumed-check-failure-notices

Conversation

@osnoser1

Copy link
Copy Markdown

A watch can announce an already consumed GitHub Actions job failure again after a pending/read fluctuation, while missing a newer job with the same check name. It remembers only the current failed names.

Retain consumed immutable failure keys in the existing watch state and carry them across rewatching. The watch's PR/repository context, observed head, run/job URL and terminal outcome identify the result; GitHub allocates a new job ID for each attempt. URL query/fragment decoration does not create another result. Hosts without immutable job URLs retain the existing transition-based behavior. Existing queued stale-head/merge guards and completion delivery remain their current owners.

Verification:

  • Extended the existing failure case in pullRequestWatch.test.ts: the consumed-job replay fails on main, then passes with the fix. It also verifies same-name new jobs/runs, a late old-job replay, and URL decoration. The complete policy file passes, 23 tests.
  • Extended the existing runtime watch case to retain the field through persisted replay and stop/rewatch. vp test run apps/server/src/orchestration-v2/runtimeLayer.test.ts -t 'starts, records, and stops a pull request watch|watching a dismissed stack layer again' passes, 2 cases.
  • The existing known completion regression passes: vp test run apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts -t 'does not re-offer when wake-policy upgrades'.
  • vp run check:changed origin/main passes format, lint, both knip checks, GraphQL, typechecks for 37 affected packages, and 211 tests in 7 files. Rebased on 371963ce0b4cfe8bac3911efd57ae3af90750b41 and reran the check on the final source.
  • Additional diagnostic selection runtimeLayer.test.ts -t 'pull request|watch' hits six existing pagination-cache assertions on both untouched main and this patch; the pagination owner/test is unchanged across the rebase. The changed persistence cases pass in isolation. Full CI remains authoritative for the complete runtime suite.

This is a server-state bug fix with no UI or user configuration change. No running T3 installation or live database was modified. History and original notifications remain available; new attempts/jobs and current-head news remain deliverable. Related to #26987's repeated wake observations; this change addresses consumed immutable check failures only.

Model: GPT-6.1 Sol, reasoningEffort=max, serviceTier=priority. Harness: Codex in T3 Code.

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

macroscopeapp Bot commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9742513

Macroscope's review found this PR approvable — Retains previously reported GitHub Actions job failures across polling fluctuations and rewatching, reducing duplicate agent wakes while distinguishing new jobs. The change is confined to watch bookkeeping and preserves reporting behavior for checks without immutable job URLs.

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

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The pull-request watch tracks reported failures by pull-request head, immutable job URL, and check status. It preserves those keys in watch state across restarts and uses the prior check-name transition rule when immutable identity information is unavailable.

Changes: Pull-request failure tracking

Layer / File(s) Summary
Identify and suppress repeated failures
packages/contracts/src/threadPullRequest.ts, apps/server/src/orchestration-v2/pullRequestWatch.ts, apps/server/src/orchestration-v2/pullRequestWatch.test.ts
The contract adds optional reportedFailures. The watch uses immutable failure keys to identify previously reported failures, with a check-name transition fallback. Tests cover pending reads, URL query and fragment variations, and changed job or run IDs.
Preserve failure history across restarts
apps/server/src/orchestration-v2/pullRequestWatch.ts, apps/server/src/orchestration-v2/PullRequestWatchReactor.ts, apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/runtimeLayer.test.ts
The watch persists reported failure keys, includes them in watch equality checks, and carries them into restarted watches. Runtime tests cover persistence and repeated failure cycles.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: Minimal · up to cf84b

This fix stops an already-reported GitHub Actions failure from being announced again after a pending/read fluctuation. It still reports newer jobs that share a check name. The change is limited to server-side watch state, and the new field is optional, so existing watches are unaffected. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title "fix(server): retain consumed immutable check failures" matches the main change. The watch now keeps consumed immutable GitHub Actions job failure keys in its state across rewatching, so a p…
Description check Passed The description follows the repository template. It has a Problem section with the bug and its effect, a Change section explaining the retained failure keys and fallback for hosts without immutable jo…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix(server): retain consumed immutable check failures" matches the main change. The watch now keeps consumed immutable GitHub Actions job failure keys in its state across rewatching, so a previously reported failure is not announced again. The title is a conventional commit, short, and specific to the server.

Full details: Description check

Explanation

The description follows the repository template. It has a Problem section with the bug and its effect, a Change section explaining the retained failure keys and fallback for hosts without immutable job URLs, a Scope section that gives context but does not link an issue, and a detailed Verification section with commands and results. It also names the model and harness at the end, as the template asks.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 9742513 and cf84ba2.

Files selected for processing: 6

  • apps/server/src/orchestration-v2/Orchestrator.ts
  • apps/server/src/orchestration-v2/PullRequestWatchReactor.ts
  • apps/server/src/orchestration-v2/pullRequestWatch.test.ts
  • apps/server/src/orchestration-v2/pullRequestWatch.ts
  • apps/server/src/orchestration-v2/runtimeLayer.test.ts
  • packages/contracts/src/threadPullRequest.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.

@github-actions

Copy link
Copy Markdown
Contributor

This PR conflicts with main. Rebase it or merge main into it. If it still conflicts after 7 days with no activity, it will be closed automatically.

See the conflict window.

@github-actions github-actions Bot added the needs-rebase Conflicts with its base branch. Closes after 7 days with no activity. label Nov 13, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

merged main without rebasing and preserved its watch baseline and pending-check behavior. added the missing reportedFailures equality comparison so existing watches persist migrated failure history instead of repeating a consumed failure notice.

the two existing recovery test cases failed before that comparison and passed afterward, including a pending-to-failure transition without another notification. blacksmith vp run check:changed github/main passed on node 26.13.1: format, lint, knip, graphql, typechecks for 37 packages, and 259 tests across seven files.

verified head cf84ba22cff05dad06b0b698cf5d7689961f3ad4: all 32 reported checks pass, github reports MERGEABLE / CLEAN, no unresolved review threads, and two independent code reviews approve this head. coderabbit reports no actionable findings. no live external failure-to-agent-wake cycle was exercised; the server runtime tests cover persistence and duplicate suppression.

@github-actions github-actions Bot removed the needs-rebase Conflicts with its base branch. Closes after 7 days with no activity. label Nov 15, 2026

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 Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants