fix(automation): replay isolated review-agent queues on current main - #835
fix(automation): replay isolated review-agent queues on current main#835seonghobae wants to merge 7 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough워크플로 전역 동시성을 제거하고 인터랙티브 멘션과 조직 스윕에 작업별 동시성 그룹을 적용했습니다. 큐 동작을 검증하는 테스트와 동시성 격리 설계·운영·롤백 문서를 추가했습니다. Changes에이전트 멘션 동시성 격리
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review Please review the unchanged exact head |
|
|
|
@coderabbitai review Re-review only the unchanged exact head |
|
I will limit the review to the bounded five-file agent-mention concurrency isolation repair. I will not use evidence from superseded
|
|
@coderabbitai review Please review only the unchanged exact head |
|
I will limit the review to the bounded five-file agent-mention concurrency-isolation repair. I will not reuse evidence from superseded PR
|
|
@coderabbitai review Please review only unchanged exact head |
|
I will limit the review to the five-file agent-mention concurrency-isolation repair. I will not use evidence from superseded PR ✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/doctoring/agent-mention-concurrency-isolation.md (1)
106-108: 🚀 Performance & Scalability | 🔵 TrivialLine 106의 큐 지연 경보에 시간 임계값을 추가하세요.
queue: max는 대기 항목 수를 최대 100개로 제한하지만, Line 106의bounded queue delay에 대한 시간 상한을 정의하지 않습니다. GitHub 문서도 이 설정을 대기 수 제한으로 설명하고 한도 초과 실행을 취소한다고 설명합니다. (docs.github.com)큐 대기시간 임계값과 한도 초과·취소 신호를 문서에 명시하세요. 그렇지 않으면 긴 대기시간을 정상 상태로 분류할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/doctoring/agent-mention-concurrency-isolation.md` around lines 106 - 108, 대화형 큐 경보 항목에 큐 대기시간의 명시적 상한을 추가하고, 100개 pending 한도 초과 및 실행 취소 신호도 함께 문서화하세요. 기존 bounded queue delay 표현을 구체적인 시간 임계값과 한도 초과 시 동작으로 갱신하되, 관련 메트릭의 유한 카디널리티 요구사항은 유지하세요.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/doctoring/agent-mention-concurrency-isolation.md`:
- Line 90: Update the Change management entry in
agent-mention-concurrency-isolation.md to mark independent review as pending
rather than completed, consistent with the stated objective that formal review
evidence is not yet available. Link the supporting evidence only after
independent review and post-integration acceptance are complete.
In `@tests/test_agent_mention_workflow_contract.py`:
- Line 83: tests/test_agent_mention_workflow_contract.py의 해당 assertion을 보강해
workflow 헤더의 루트 concurrency 키를 모든 위치에서 검출하세요. 첫 줄의 concurrency:와 concurrency:
<value> 같은 매핑·스칼라 형식을 모두 거부하도록 키 경계와 값 형식을 함께 검사하고, 기존 허용 범위는 유지하세요.
---
Nitpick comments:
In `@docs/doctoring/agent-mention-concurrency-isolation.md`:
- Around line 106-108: 대화형 큐 경보 항목에 큐 대기시간의 명시적 상한을 추가하고, 100개 pending 한도 초과 및
실행 취소 신호도 함께 문서화하세요. 기존 bounded queue delay 표현을 구체적인 시간 임계값과 한도 초과 시 동작으로 갱신하되,
관련 메트릭의 유한 카디널리티 요구사항은 유지하세요.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6d9dd78-a578-46b7-982c-336b13bc6fe7
📒 Files selected for processing (5)
.github/workflows/agent-mention-router.ymlCHANGELOG.mddocs/doctoring/agent-mention-concurrency-isolation.mdtests/test_agent_mention_artifact_ledger.pytests/test_agent_mention_workflow_contract.py
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review Please perform a fresh current-head review of exact head |
|
I will limit the review to the five-file concurrency-isolation repair. I will assess the strengthened root-concurrency regression and the corrected doctoring. I will use current source evidence only and will not reuse conclusions from predecessor PR ✅ Action performedReview finished.
|
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: agent-mention-router.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: agent-mention-router.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: agent-mention-concurrency-isolation.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: agent-mention-concurrency-isolation.md"]
R3 --> V3["docs review"]
Evidence --> S4["Test (2 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (2 files)"]
R4 --> V4["targeted test run"]
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
APPROVE of the bounded five-file concurrency-isolation repair. Changed-file evidence inspected: .github/workflows/agent-mention-router.yml, tests/test_agent_mention_workflow_contract.py, docs/doctoring/agent-mention-concurrency-isolation.md, CHANGELOG.md, tests/test_agent_mention_artifact_ledger.py. Approval sufficiency: the PR intent (separate job-scoped concurrency groups so scheduled sweeps cannot evict pending interactive trusted-mention routes; queue: max on the local route, cancel-in-progress: false on the sweep) is directly implemented in the current-head workflow and locked by contract tests. Verification posture: trusted static contract tests test_interactive_mentions_and_sweeps_have_independent_queue_contracts and test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes assert the exact local and sweep concurrency blocks and reject any workflow-level concurrency key; the supported suite python3 -m pytest tests passed (Coverage execution evidence Result: PASS, supported repository test suites passed). Linter/static: no failed GitHub Checks were present at current head and no lint failures appear in evidence. TDD/regression: two workflow contract tests plus the ledger-doc contract test in tests/test_agent_mention_artifact_ledger.py cover the changed surfaces. Coverage: Coverage execution evidence Result: PASS, supported repository test suites passed. Docstring coverage: configured repository docstring gates passed or were advisory per Coverage execution evidence. DAG: CodeGraph current-head evidence (138 files, 2870 nodes) shows the workflow-contract constants are consumed only by the contract tests; the Mermaid flowchart below maps agent-mention-router.yml to the two jobs, their independent concurrency groups, the main risk, and the verification path; it reflects the head flow. PoC/execution: no execution receipts beyond the trusted coverage run; YAML queue semantics are enforced by the passing static contract suite. DDD/domain: CI automation domain; workflow-level concurrency removed and job-scoped queues introduced; changelog and doctoring doc are consistent with the YAML. CDD/context: the doctoring doc records the incident RCA, fail-first evidence (RED head a319d51), the decision, security/privacy boundaries, monitoring, and rollback criteria. Similar issues: predecessor PR #825/#824 evidence is explicitly not reused, and CodeRabbit's two actionable comments on predecessor head 4881c78 are not present as unresolved threads at current head 6a1acb1. Claim/concept check: queue: max (up to 100 pending, serial processing, incompatible with cancel-in-progress: true) matches the GitHub concurrency contract recorded in the doctoring doc, and the YAML never combines the two keywords. Standards search: GitHub Actions concurrency semantics per the repository doctoring record; external lookup is unavailable to the isolated model. Compatibility/convention: the new identifiers review-agent-mention-router-local and review-agent-mention-router-sweep are multi-word kebab-case with no reserved-word or serialization risk; no DB/API/config objects changed; workflow default permissions remain contents: read with unchanged job-scoped writes. Breaking-change/backcompat: the concurrency behavior change is intentional, release-noted in CHANGELOG.md, and alters no consumer API. Performance: queue: max bounds interactive pending runs at the platform limit while the sweep retains single-pending coalescing; the bounded 5/15-minute job timeouts are unchanged. Developer experience: operator-facing CI behavior, specifically trusted @mention acknowledgement that is no longer silently evicted; the doctoring doc adds queue-delay, receipt-delay, sweep-duration, and duplicate-claim monitoring with acceptance criteria. User experience: non-web surface, the @cwl-noema-review and @opencode-agent mention router users and operators; judged from the workflow YAML, doctoring doc, and changelog. Visual/DOM: non-web interaction surface (GitHub Actions concurrency and runtime queue behavior) reviewed; no web UI exists and no Playwright/DOM evidence applies. Accessibility/i18n: no UI change; the doc is English with one Korean date header line (non-blocking). Supply-chain/license: no dependency or package changes in this PR. Packaging: no manifests or package contracts changed; scripts untouched. Security/privacy: the least-privilege trigger guard is unchanged (non-bot OWNER/MEMBER/COLLABORATOR comments on pull requests in the central repository only), no new secrets are introduced, tokens remain github.token, workflow permissions stay contents: read, there is no sequential-id or identifier-exposure surface, and the exact-name artifact ledger retention and metadata minimization are preserved. No blocking findings.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/agent-mention-router.yml, CHANGELOG.md, docs/doctoring/agent-mention-concurrency-isolation.md, tests/test_agent_mention_artifact_ledger.py, tests/test_agent_mention_workflow_contract.py.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/agent-mention-router.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":".github/workflows/agent-mention-router.yml","line":27,"hypothesis":"The concurrency fix was not applied: the workflow still declares one shared workflow-level concurrency group, or the local job lacks queue: max, so a scheduled sweep can still evict a pending interactive mention before routing.","attack_or_counterexample":"Replay the original incident: a trusted issue_comment route is queued while the every-five-minute organization sweep starts; with a shared group and a single-pending queue the sweep replaces the pending route before exact-head resolution.","evidence":"Trusted source trace at .github/workflows/agent-mention-router.yml:27 shows the concurrency key nested under job route-local-agent-mention with group review-agent-mention-router-local-${{ github.repository }} and queue: max, and the workflow header contains no concurrency key; the static contract test test_interactive_mentions_and_sweeps_have_independent_queue_contracts in tests/test_agent_mention_workflow_contract.py passed in the supported suite (Coverage execution evidence Result: PASS, supported repository test suites passed), asserting the exact local concurrency block and rejecting any header line starting with concurrency:. source-line-sha256=a399531b10a56a1ae2ddcb1a67a4c9cd9b68fdabafebebbd2c8ee98301a14c82","outcome":"falsified"},{"path":".github/workflows/agent-mention-router.yml","line":74,"hypothesis":"The sweep job lost its non-cancelling protection or shares the local group, so either a running sweep can be cancelled mid-dispatch or a long sweep blocks interactive routes.","attack_or_counterexample":"Start a 15-minute sweep and then trigger a trusted interactive mention: if the two jobs shared one group, the interactive route would wait behind or be evicted by the sweep; if cancel-in-progress were true, the running sweep would be cancelled mid-dispatch.","evidence":"Trusted source trace at .github/workflows/agent-mention-router.yml:74 shows cancel-in-progress: false under job sweep-organization-agent-mentions with the distinct group review-agent-mention-router-sweep-${{ github.repository }} on line 73, so the sweep is neither cancellable nor part of the local group; the same contract test passed in the supported suite (Coverage execution evidence Result: PASS, supported repository test suites passed), asserting the exact sweep concurrency block and independent local/sweep groups. source-line-sha256=0a154ba3f27baabda66de506a511243140983e00ab40cb1a37e9bb7983bdc2d2","outcome":"falsified"}],"residual_risk":"GitHub platform-side acceptance of job-level queue: max and the exact 100-pending limit cannot be proven from local evidence (isolated model, no network); the YAML is internally consistent (queue: max is never combined with cancel-in-progress) and matches the contract recorded in the repository doctoring doc, and no failed GitHub Checks were present at current head. The truncated tail of the new doctoring doc (approximately lines 100-124) and the precise one-line diff of tests/test_agent_mention_artifact_ledger.py were not fully inlined in the evidence packet; current-head CodeGraph source (DOC constant at tests/test_agent_mention_artifact_ledger.py:17 pointing at the new doc) and the passing ledger-doc contract test substantiate both. Direct file reads of the head checkout were unavailable to the model, so the CodeGraph verbatim source blocks and focused hunks were used as the authoritative current-head source."}- Result: APPROVE
- Reason: Concurrency isolation is implemented exactly as intended at current head: the workflow-level concurrency group is removed, independent job-scoped groups are added (queue: max for interactive routes, cancel-in-progress: false for the sweep), the exact YAML shape is locked by passing contract tests, and the change is documented in CHANGELOG.md and the new doctoring record. No blocking findings.
- Head SHA:
6a1acb1559755c27d7267e39fd6275bb9db74dc9 - Workflow run: 31302651908
- Workflow attempt: 1
1 participant
RCA
Protected
mainroutes trusted interactiveissue_commentrequests and the five-minute organization sweep through one workflow-level concurrency group. GitHub permits one running and, by default, one pending member in a concurrency group; newer queued work can replace an existing pending run even whencancel-in-progressis false. A scheduled sweep can therefore discard a pending trusted@cwl-noema-reviewor review-only@opencode-agentrequest before exact-head resolution, durable-ledger claim, dispatch, or acknowledgement.Historical PR #825 contains the bounded repair but is no longer mergeable and is based on protected-main
1131b1bbafb24e455fc8619cdf316813e8721861. Protectedmainadvanced to6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba. No #825 check, review, approval, comment, or synthetic-merge evidence transfers here.Feasibility decision
A destructive rebase/force-push is unnecessary. The old base → current-main delta modifies only the Strix quality lane, its doctoring/test, and one independent changelog entry; it does not touch the mention router or its tests. This replacement is rebuilt directly from exact current protected main and preserves the bounded five-file repair plus the intervening main changelog entry.
Bounded repair
review-agent-mention-router-local-${{ github.repository }}withqueue: max;review-agent-mention-router-sweep-${{ github.repository }}withcancel-in-progress: false;Exact replacement boundary
This branch is exactly five commits ahead and zero behind protected main at creation. The effective diff is limited to:
.github/workflows/agent-mention-router.ymltests/test_agent_mention_workflow_contract.pytests/test_agent_mention_artifact_ledger.pydocs/doctoring/agent-mention-concurrency-isolation.mdCHANGELOG.mdNo predecessor evidence transfers. Keep Draft until implementation/tests/docs/changelog are complete and every currently executable exact-head deterministic/security/supply-chain gate is green with zero current source-backed findings. At that point Ready status is permitted solely to obtain current-head automated and independent review; Ready is not merge evidence. Merge remains prohibited until current automated review is complete, zero valid unresolved findings remain, a qualifying non-author counted approval exists, and branch protection/repository policy allow the exact unchanged head.
Operational acceptance
After protected integration, post fresh trusted Noema/OpenCode review-only comments and require durable receipt/acknowledgement and downstream dispatch. Verify a scheduled sweep cannot replace an interactive route. Static PR checks alone are not incident closure.
Supersedes #825 only after this replacement proves its own exact-head gates; #814 closes only after protected-main operational evidence.
Summary by CodeRabbit
개선 사항
문서
테스트