Skip to content

[Feat] Include CI status in PR review feedback thread replies - #162

Merged
mrubens merged 6 commits into
developfrom
feature/pr-review-ci-status-sentence-185unvkkk9ixp
Jul 11, 2026
Merged

[Feat] Include CI status in PR review feedback thread replies#162
mrubens merged 6 commits into
developfrom
feature/pr-review-ci-status-sentence-185unvkkk9ixp

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Opened on behalf of Matt Rubens. Follow up by mentioning @openmote, in the web UI, or in Slack.

Important

Roomote does not generally accept unsolicited community pull requests. Unless a maintainer invited this PR in an issue or discussion, it will likely be closed without review. A well-written issue is the best way to contribute.

Related issue

Internal request from Slack (PR review notification thread).

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

What changed

When Roomote posts PR review feedback into the originating chat thread (Slack, Teams, or Telegram), the triage LLM that writes that message now receives the live CI check state for the PR head.

CI is fetched from GitHub check runs plus classic combined commit status and passed into the prompt as structured context (CI overall status, and a failed check name when relevant). Empty classic status lists (pending + total_count: 0, the Actions-only case) are treated as unavailable so green check runs are not overridden. The model is instructed to weave one short natural CI sentence into the notification when that context is present, rather than hardcoding an appended sentence after the summary.

How it was tested

  • Unit coverage for CI context gathering (green/pending/failure/GitLab skip/empty combined status) and prompt inclusion in pr-review-notification-delivery.test.ts
  • pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/sdk exec vitest run src/server/lib/task-runs/__tests__/pr-review-notification-delivery.test.ts
  • Pre-push lint/typecheck/knip passed

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

mrubens added 2 commits July 11, 2026 02:48
Append a short current CI sentence to PR review feedback posts so chat threads show green, running, or failed check context.
Keep CI status fetch and types internal so pre-push knip stays clean.
@roomote-roomote

roomote-roomote Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

No new code issues found. The latest change keys CI check dedup on the full check/context name and merges same-name collisions by severity, so distinct checks that share a leaf segment (for example CI / Lint and Docs / Lint) stay separate. Both earlier findings remain resolved. See task

  • packages/sdk/src/server/lib/task-runs/pr-review-notification-delivery.ts:297 - Green CI on GitHub Actions-only repos was reported as "Tests are still running." because the classic combined commit status returns state: "pending" when there are no classic statuses. Now empty combined status (total_count: 0) is treated as unavailable.
  • packages/sdk/src/server/lib/task-runs/pr-review-notification-delivery.ts:170 - collectCiChecks previously keyed the dedup map on the shortened leaf name, collapsing distinct checks that share a leaf segment (e.g. CI / Lint and Docs / Lint) and unconditionally overwriting earlier entries. Now keyed on the full check/context name with a severity-preferring merge on same-name collisions.

@roomote-roomote

roomote-roomote Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review finding in 5da96a7.

CI check lines are now deduped by the full check/context name (so CI / Lint and Docs / Lint both appear), and same-name collisions prefer the more severe status. Also updated unit coverage for the leaf-collision case.

mrubens added 4 commits July 11, 2026 02:54
Do not treat pending+empty classic statuses as still-running, so green
check runs report CI is green.
Stop appending a hardcoded CI sentence. Pass overall status and any
failed check name into the triage prompt so the LLM can include CI
context in natural message wording.
Give the notification LLM one line per check (name: status) instead of
an overall status rollup, using Actions check runs and classic statuses.
Key CI check dedupe by the full check/context name so `CI / Lint` and
`Docs / Lint` both stay in LLM context, preferring more severe status on
same-name collisions.
@mrubens
mrubens marked this pull request as ready for review July 11, 2026 04:46
@mrubens
mrubens merged commit bd6b578 into develop Jul 11, 2026
1 check passed
@mrubens
mrubens deleted the feature/pr-review-ci-status-sentence-185unvkkk9ixp branch July 11, 2026 04:46
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