Skip to content

fix(automation): reuse review node_id for mention eyes - #991

Draft
seonghobae wants to merge 11 commits into
mainfrom
fix/mention-router-review-node-id
Draft

fix(automation): reuse review node_id for mention eyes#991
seonghobae wants to merge 11 commits into
mainfrom
fix/mention-router-review-node-id

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

Submitted review-body mentions now react through GraphQL addReaction, but the router always GET /pulls/{n}/reviews/{id} first. Live webhooks and the five-minute sweep already return node_id on the review object, so that extra GET is wasted and can fail independently after dispatch.

This increment copies node_id onto MentionRequest when present and skips the review GET. Blank or non-string values still fall back to the lookup.

Verification

  • Sweep fixture with node_id on the submitted review yields request.review_node_id. Cached-node reaction posts only graphql and never GET /reviews/.
  • coverage run -m pytest tests && coverage report --show-missing twice at 100% (988 passed, scripts/ci 100% statement/branch), then interrogate 100%.

@cwl-noema-review

@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a1f32a31-85ce-4706-a0f0-e0429804f192

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@seonghobae
seonghobae enabled auto-merge (squash) August 13, 2026 11:29
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head 0ce9c707 — reuse review node_id for mention eyes. Head moved after the earlier mention. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head 53ff111c — reuse review node_id for mention eyes; drop invalid reactions:write mention scope. Head moved after 0ce9c707. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

@seonghobae
seonghobae force-pushed the fix/mention-router-review-node-id branch from 53ff111 to 06a3b13 Compare August 13, 2026 18:37
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head 06a3b135 — reuse review node_id for mention eyes; drop invalid reactions:write; coverage accepts only bounded relative requirement includes. Head moved after 53ff111c. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head 1692c842 — reuse review node_id for mention eyes; drop invalid reactions:write. Head moved after 06a3b135. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

@seonghobae
seonghobae marked this pull request as draft August 14, 2026 09:16
auto-merge was automatically disabled August 14, 2026 09:16

Pull request was converted to draft

Copy link
Copy Markdown
Contributor Author

Designated as the canonical cumulative review-mention surface branch and returned to Draft.

Its current production tree already includes the earlier review-comment/review-body dispatch support, correct REST reaction endpoint for inline review comments, GraphQL review-body eyes with idempotent already-reacted handling, and the node_id fast path with lookup fallback. Therefore #954, #989, and #990 are predecessor slices rather than independent merge units.

Before Ready, rebuild from protected main and remove the unrelated trusted-uv materializer source/tests. Retain only the mention workflows, router/sweep implementation, prompts directly required by the invocation contract, focused webhook/sweep/reaction/idempotency/node-ID regressions, doctoring/governance/changelog evidence. Update the body to describe the complete cumulative buyer-visible behavior, then reacquire exact-head router/security/supply-chain and independent-review evidence.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Rebased onto origin/main c47bee5 and pushed exact HEAD e1fe193. Verified 89 focused mention-router/sweep/workflow/OpenCode/idempotency tests, compileall, interrogate 100%, and git diff --check. The local actionlint binary rejects hosted queue:max syntax in the two touched workflows; hosted workflow validation remains authoritative. Please review this exact head.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review exact current head e1fe193 against base c47bee5. Review every changed file, preserve review-only mode, and report terminal evidence.

Trusted @cwl-noema-review and @opencode-agent mentions on pull-request
review comments and submitted reviews were ignored because parse_event
required issue.pull_request. Accept those surfaces, sweep them, drop the
case-sensitive workflow body filter, and require a per-changed-file
review walk with staged compute allocation.
Live run 31670687388 queued @cwl-noema-review for #954 then failed the
job on POST .../reactions with 403 Resource not accessible by
integration, so no receipt was posted. Treat the reaction as optional,
give the local job pull-requests: write for conversation receipts, and
keep durable dispatch evidence authoritative.
Record that an exceptional eyes-reaction response cannot be treated as a
missed dispatch. Force the trusted-uv installer tests onto the linux
x86_64 runner path and add the control-plane architecture diagram.
Doctoring already required skipping pending reviews, but parse_event
and the sweep treated a dismissed body with submitted_at as a live
mention. Dispatch only submitted, non-dismissed review bodies.
route-local-agent-mention on main dies after a successful dispatch
because GITHUB_TOKEN lacks the reactions permission (HTTP 403).
Keep the reaction non-fatal and declare the write so the eyes
receipt can actually appear.
Inline @cwl-noema-review mentions now POST /pulls/comments/{id}/reactions.
A 403 there is a warning after dispatch. Submitted review bodies still
have no REST reaction endpoint.
Submitted @cwl-noema-review review bodies have no REST reaction
endpoint. Resolve the review node_id and addReaction EYES. A 403 or
GraphQL error is a warning after dispatch.
Webhook and sweep review payloads already include node_id. Reuse it
for GraphQL addReaction so recovered review-body mentions skip an
extra review GET.
A second mention on the same submitted review returns a GraphQL
already-reacted error. That is still eyes on the review. An empty
addReaction payload is not.
node_id reuse heads still declared a non-existent GITHUB_TOKEN scope.
Issue-comment eyes use issues: write; review-comment eyes use
pull-requests: write. NVIDIA_NIM_API_KEY only.
@seonghobae
seonghobae force-pushed the fix/mention-router-review-node-id branch from e1fe193 to 1fa547a Compare August 20, 2026 06:10
@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-current-head repair loop: rebased the cumulative mention-routing surface onto protected main 2cce96f, preserving the review-node-ID fast path, GraphQL review-body reaction handling, cosmetic reaction fail-open behavior, durable existing-ledger acknowledgement, and no invalid reactions:write scope. Current head 1fa547a. Focused mention-router/sweep/workflow/OpenCode suite: 89 passed; interrogate, compileall, and diff-check passed. Hosted Checks are regenerating; keep Draft and review this exact head only.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Resolved the exact-head queue isolation failure: removed the workflow-wide concurrency group and replaced the unsupported key with valid job-scoped non-cancelling concurrency. Targeted tests (85) and actionlint pass; the new exact-head Checks are running.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Correction: resolved the exact-head queue isolation failure by removing the workflow-wide concurrency group and replacing the unsupported queue key with valid job-scoped non-cancelling concurrency. Targeted tests (85) and actionlint pass. New exact-head Checks are running.

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