Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/workflows/agent-mention-router.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ on:
schedule:
- cron: "*/5 * * * *"

concurrency:
group: review-agent-mention-router-${{ github.repository }}
cancel-in-progress: false

# Organization required-workflow rules do not propagate issue_comment events
# into sibling repositories. Keep the workflow default read-only; each bounded
# job declares only the writes it actually needs.
Expand All @@ -28,6 +24,9 @@ jobs:
contains(github.event.comment.body, '@cwl-noema-review')
|| contains(github.event.comment.body, '@opencode-agent')
)
concurrency:
group: review-agent-mention-router-local-${{ github.repository }}
queue: max
runs-on: ubuntu-24.04
timeout-minutes: 5
permissions:
Expand Down Expand Up @@ -70,6 +69,9 @@ jobs:
if: >-
github.repository == 'ContextualWisdomLab/.github'
&& github.event_name == 'schedule'
concurrency:
group: review-agent-mention-router-sweep-${{ github.repository }}
cancel-in-progress: false
runs-on: ubuntu-24.04
timeout-minutes: 15
permissions:
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release.

### Fixed

- Isolated trusted issue-comment routing from scheduled organization sweeps with job-scoped concurrency, ordered bounded local queuing, coalesced sweep queuing, exact-key durable duplicate suppression, and permanent fail-first and doctoring evidence so scheduled work cannot silently replace pending review requests.
- Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped.
- Bound both trusted-uv quality jobs to `github.event.pull_request.head.sha` and added a permanent two-checkout regression contract so exact-head compatibility, coverage, docstring, and compilation claims cannot silently measure GitHub's generated pull-request merge revision.
- Made Strix treat only a single LiteLLM provider-error line containing NVIDIA NIM context and model-catalog 404 evidence as cross-model fallback evidence, rejecting cross-line signal assembly and provider-like target source literals; moved the public default to Nemotron 3 Super 120B and added a second NVIDIA hosted candidate before GitHub Models without neutralizing reported vulnerabilities.
- Made Strix treat only a single LiteLLM provider-error line containing NVIDIA NIM context and model-catalog 404 evidence as cross-model fallback evidence, rejecting cross-line signal assembly and provider-like target source literals; moved the public default to Nemotron 3 Super 120B and added a second NVIDIA hosted candidate before GitHub Models without neutralizing reported vulnerabilities.
144 changes: 144 additions & 0 deletions docs/doctoring/agent-mention-concurrency-isolation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# Review-agent mention concurrency isolation

## Incident

Trusted `@cwl-noema-review` and review-only `@opencode-agent` comments could remain unacknowledged even though the protected-main router was enabled. Repeated exact-head requests on `.github#813` and `mightyETL#121` produced neither the durable router receipt nor the expected `eyes` acknowledgement.

The failure occurred before model execution. The central workflow mixed two event classes in one workflow-level concurrency group:

- an interactive `issue_comment` route with a five-minute job timeout; and
- an organization-wide sweep scheduled every five minutes with a fifteen-minute job timeout.

GitHub Actions permits one running member and, by default, one pending member in a concurrency group. A new queued member replaces the existing pending member even when `cancel-in-progress` is false. A scheduled sweep could therefore replace a pending trusted comment run before it read the comment, resolved the exact pull-request head, dispatched a reviewer, wrote the durable invocation ledger, or acknowledged the request.

This was a deterministic queue configuration defect, not a model, credential, allowlist, or review-quality failure.

## Fail-first evidence

Draft pull request #815 first added only the permanent regression contract at exact head `ca9a03109428332b4c35f4b24313580eda5cd92c`.

Agent Mention Router Quality CI run `31154969412`, job `92792374352`, executed the complete repository suite and produced the intended result:

```text
1 failed, 963 passed
FAILED tests/test_agent_mention_workflow_contract.py::
test_interactive_mentions_and_sweeps_have_independent_queue_contracts
```

The failure proved that the inherited workflow still contained the shared top-level group. No test was weakened or skipped before the production change.

## Decision

Move concurrency from the workflow to the two jobs and give each event class a separate group.

```yaml
route-local-agent-mention:
concurrency:
group: review-agent-mention-router-local-${{ github.repository }}
queue: max

sweep-organization-agent-mentions:
concurrency:
group: review-agent-mention-router-sweep-${{ github.repository }}
cancel-in-progress: false
```

The interactive group uses `queue: max`. GitHub currently permits up to 100 pending members in that mode and processes waiting members serially. It is deliberately not combined with `cancel-in-progress: true`, which GitHub rejects as contradictory.

The sweep group retains the default single-pending behavior. A running sweep is not interrupted, but newer schedules may coalesce obsolete pending sweeps. This avoids an unbounded maintenance backlog while preventing scheduled work from replacing an interactive request.

Local and sweep jobs may overlap because they now use different groups. Duplicate forwarding is still prevented by the existing deterministic invocation key, exact-key downstream concurrency, and immutable exact-name Actions artifact ledger written before authoritative forwarding.

## Data and authority flow

```mermaid
sequenceDiagram
participant M as Trusted maintainer
participant L as Local comment queue
participant S as Scheduled sweep queue
participant R as Central router
participant D as Exact-key downstream dispatcher
participant A as Durable artifact ledger
participant V as Review workflow

M->>L: issue_comment with exact-head request
S->>R: bounded organization scan
L->>R: ordered interactive request
R->>D: canonical invocation payload and SHA-256 key
D->>A: claim exact ledger name
alt ledger claim succeeds
D->>V: forward once to authoritative reviewer
R-->>M: durable receipt and acknowledgement
else ledger already exists
D-->>R: duplicate suppressed
end
```

The receipt proves that routing and durable claim processing occurred. It is not an approval and does not weaken exact-head review, required checks, branch protection, or expected-head merge rules.

## Preserved security boundaries

- No model provider, reviewer identity, token name, secret, repository allowlist, dispatch payload, or permission is changed.
- `COPILOT_GITHUB_TOKEN` remains unused.
- The workflow default remains `contents: read`; each job retains only its existing job-scoped writes.
- The local route still accepts only non-bot `OWNER`, `MEMBER`, or `COLLABORATOR` comments on pull requests in the central repository.
- The sweep still uses the configured organization token or bounded OpenCode installation-token exchange for sibling-repository reads.
- Exact pull-request number, base branch, current head SHA, requesting actor, source comment identifier, and requested agent remain bound into the canonical invocation key.
- Artifact-ledger creation remains the authority for idempotent forwarding. Concurrency alone is not treated as durable uniqueness.
- Ordinary logs and queue evidence exclude comment bodies, model output, private tokens, raw credentials, and repository data beyond bounded identifiers already present in GitHub Actions metadata.

## CSAP and SOC 2 operating evidence

The repair supports availability and processing-integrity control evidence without claiming certification from code alone.

| Control concern | Evidence |
| --- | --- |
| Change authorization | Protected pull request, exact-head checks, independent review, and immutable commit history |
| Availability | Separate interactive and sweep groups, bounded job timeouts, and queued interactive requests |
| Processing integrity | Deterministic invocation key, exact-name artifact claim, duplicate suppression, and receipt semantics |
| Security | Existing least-privilege job permissions, secret separation, no new token, and default-branch trusted code |
| Monitoring | Workflow conclusion, queue delay, receipt delay, sweep duration, dispatch count, and duplicate-claim outcome |
| Incident response | This doctoring record, fail-first run identifiers, rollout checks, and rollback constraints |
| Privacy | Architectural data minimization; the router needs metadata, not business payloads or PII |

The alternative to PII masking is separation: this automation path does not read business payloads at all. It uses purpose-bound metadata, encrypted GitHub transport and storage, role-based repository access, and **30-day artifact retention** (`retention-days: 30`) for the bounded invocation ledger.

Operators inspect ledger liveness and expiry through GitHub's *REST API endpoints for GitHub Actions artifacts*. Upload and retention behavior follows *Store and share data with workflow artifacts*. The ledger contains bounded invocation metadata, not comment bodies, model output, credentials, or business data.

## Monitoring and acceptance

After protected merge:

1. submit one fresh exact-head Noema request on `.github#813` and one on `mightyETL#121`;
2. require a durable receipt marker or acknowledgement before relying on downstream review evidence;
3. verify that no interactive router run is canceled by a scheduled sweep;
4. inspect local queue delay, sweep duration, dispatch count, duplicate-ledger outcomes, and downstream workflow conclusions;
5. alert when an eligible comment has no receipt within the local five-minute timeout plus bounded queue delay;
6. alert when the interactive queue approaches its 100-pending platform limit or when sweep duration repeatedly exceeds its five-minute cadence;
7. keep metrics finite-cardinality and exclude comment text, pull-request diff content, tokens, and model responses.

A review workflow may still fail closed because credentials, providers, checks, or exact-head evidence are unavailable. That is distinct from an unacknowledged router invocation and must remain visible.

## Rollback

Rollback requires an independently reviewed replacement that proves scheduled work cannot replace pending interactive requests. Restoring the shared workflow-level group is not acceptable.

A safe emergency degradation is to suspend the scheduled sweep while retaining the isolated local queue. Removing `queue: max` from the local group is unsafe unless another durable queue preserves every eligible comment invocation.

## APA 7th references

GitHub. (2026a). *Concurrency*. GitHub Docs. https://docs.github.com/en/actions/concepts/workflows-and-actions/concurrency

GitHub. (2026b). *Control the concurrency of workflows and jobs*. GitHub Docs. https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency

GitHub. (2026c). *Events that trigger workflows*. GitHub Docs. https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows

GitHub. (2026d). *Troubleshooting workflows*. GitHub Docs. https://docs.github.com/en/actions/how-tos/troubleshoot-workflows

GitHub. (2026e). *REST API endpoints for GitHub Actions artifacts*. GitHub Docs. https://docs.github.com/en/rest/actions/artifacts

GitHub. (2026f). *Store and share data with workflow artifacts*. GitHub Docs. https://docs.github.com/en/actions/tutorials/store-and-share-data

Korea Internet & Security Agency. (2025). *2025 cloud service security assurance program guide*. https://isms.kisa.or.kr/main/csap/notice

American Institute of Certified Public Accountants. (2023). *2017 Trust Services Criteria for security, availability, processing integrity, confidentiality, and privacy (with revised points of focus—2022)*. AICPA & CIMA. https://www.aicpa-cima.com/resources/download/2017-trust-services-criteria-with-revised-points-of-focus-2022
2 changes: 1 addition & 1 deletion tests/test_agent_mention_artifact_ledger.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
MODULE_PATH = ROOT / "scripts" / "ci" / "agent_mention_router.py"
NOEMA_WORKFLOW = ROOT / ".github" / "workflows" / "agent-mention-noema-dispatch.yml"
OPENCODE_WORKFLOW = ROOT / ".github" / "workflows" / "agent-mention-opencode-dispatch.yml"
DOC = ROOT / "docs" / "automation" / "review-agent-comment-invocation.md"
DOC = ROOT / "docs" / "doctoring" / "agent-mention-concurrency-isolation.md"
UPLOAD_ARTIFACT_SHA = "043fb46d1a93c77aae656e7c1c64a875d1fc6a0a"


Expand Down
42 changes: 42 additions & 0 deletions tests/test_agent_mention_workflow_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,23 @@
CHECKOUT_PIN = "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1"


def _job_block(workflow: str, job_name: str, next_job_name: str | None) -> str:
"""Return one top-level job block bounded by the following job."""
jobs = workflow.split("\njobs:\n", 1)[1]
start = jobs.index(f" {job_name}:\n")
if next_job_name is None:
return jobs[start:]
end = jobs.index(f"\n {next_job_name}:\n", start)
return jobs[start:end]


def _concurrency_block(job: str) -> str:
"""Return the exact job-scoped concurrency mapping before ``runs-on``."""
start = job.index(" concurrency:\n")
end = job.index("\n runs-on:", start)
return job[start:end]


def test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes() -> None:
"""The router is central-only, scheduled, and least-privileged."""

Expand Down Expand Up @@ -51,6 +68,31 @@ def test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes() ->
assert "agent_mention_sweep.py" in sweep


def test_interactive_mentions_and_sweeps_have_independent_queue_contracts() -> None:
"""Scheduled sweeps cannot replace a pending trusted comment invocation."""

text = WORKFLOW.read_text(encoding="utf-8")
header = text.split("\njobs:\n", 1)[0]
local_job = _job_block(
text,
"route-local-agent-mention",
"sweep-organization-agent-mentions",
)
sweep_job = _job_block(text, "sweep-organization-agent-mentions", None)

assert "\nconcurrency:\n" not in header
assert _concurrency_block(local_job) == (
" concurrency:\n"
" group: review-agent-mention-router-local-${{ github.repository }}\n"
" queue: max"
)
assert _concurrency_block(sweep_job) == (
" concurrency:\n"
" group: review-agent-mention-router-sweep-${{ github.repository }}\n"
" cancel-in-progress: false"
)


def test_quality_workflow_measures_exact_files_without_module_name_warnings() -> None:
"""Coverage includes the two script paths instead of treating paths as modules."""

Expand Down
Loading