Skip to content

docs(agents): code-owner gates disabled org-wide until multiple maintainers exist - #208

Open
seonghobae wants to merge 4 commits into
masterfrom
cwl/codeowner-policy-note
Open

docs(agents): code-owner gates disabled org-wide until multiple maintainers exist#208
seonghobae wants to merge 4 commits into
masterfrom
cwl/codeowner-policy-note

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

Records the 2026-08-04 org policy in AGENTS.md: code-owner review requirements are disabled everywhere (solo maintainer cannot satisfy them). ON HOLD until the org has multiple maintainers — do not re-enable.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c9e85ef1-c674-4f8f-8302-1d4e844abbea

📥 Commits

Reviewing files that changed from the base of the PR and between 35e4498 and 38a53ac.

📒 Files selected for processing (2)
  • .github/workflows/r.yml
  • AGENTS.md

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai approve

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

Seongho Bae and others added 2 commits August 4, 2026 13:46
…arallel 6.2

The check job fails building SimDesign because the P3M stringfish binary
references legacy tbb::internal symbols removed from the oneTBB shipped
by the newly released RcppParallel 6.2.0 (undefined symbol
_ZN3tbb8internal25concurrent_vector_base_v316internal_grow_by...).
Forcing cran::stringfish?source makes pak compile it against the
installed RcppParallel so the symbols match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cran::stringfish?source ref did not stick (pak resolves the
transitive dep to the P3M binary), and here the broken binary kills the
SimDesign source build inside setup-r-dependencies itself, so the
rebuild must run before that step. Pre-install Rcpp/RcppParallel, then
build stringfish from cloud.r-project.org source (P3M serves binaries
even through its source API); pak keeps the installed same-version copy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@opencode-agent

opencode-agent Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 38a53acec6e527117ab78eb2c1871b15d67b9a28
  • Workflow run: 31222594861
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode reviewed the current-head bounded evidence and requested changes before merge.

Findings

1. P2 .github/workflows/r.yml:38 - Unannounced CI workaround bundled into a docs-only PR with an unverified correctness premise

  • Problem: PR title 'docs(agents): code-owner gates disabled org-wide until multiple maintainers exist' and body describe only the AGENTS.md policy note, yet the diff also adds a 'Prebuild stringfish from source (TBB mismatch with RcppParallel 6.2.0)' step that changes every R check run. Failed GitHub Check evidence reports 'No completed failed GitHub Checks were present when evidence was collected' and no OPENCODE_EXECUTION_RECEIPT exists for the step at head SHA 38a53ac, so neither the claimed RcppParallel 6.2.0/stringfish TBB load failure nor the workaround's effectiveness is corroborated.
  • Root cause: Docs-only PR scope bundled an unverified CI workaround without announcing it in the title/body and without attaching completed check evidence for the head SHA.
  • Fix: Split the workflow change into a separate CI-focused PR, or update this PR's title/body to announce the workflow change and attach the completed successful R check run for this head SHA; keep AGENTS.md documentation change separate if preferred.
  • Regression test: Run the repository R check workflow (r-lib/actions/setup-r-dependencies then r-lib/actions/check-r-package) for head SHA 38a53ac and attach the completed check log proving R CMD check passes with the prebuilt stringfish.
  • Suggested diff: posted in this finding's inline review thread.

2. P2 .github/workflows/r.yml:38 - Floating-version prebuild runs before setup-r-dependencies and can be silently defeated on cache-hit runs

  • Problem: The step installs Rcpp, RcppParallel, and stringfish via install.packages with no version constraints and runs before r-lib/actions/setup-r-dependencies, which manages its own pak-based library and cache. The step comment asserts 'pak then keeps the installed copy', but the workflow pins nothing and verifies nothing after dependency setup; on a cache-hit or re-resolution run setup-r-dependencies can restore or reinstall the P3M binary stringfish, so the step's green result does not prove the load-time TBB mismatch is gone.
  • Root cause: No version pins, no guarantee the prebuilt library is the library setup-r-dependencies resolves, and no post-setup verification step that asserts stringfish loads from the source build.
  • Fix: Pin exact versions (RcppParallel 6.2.0 and the matching stringfish release), install into the same library target that setup-r-dependencies uses (or declare the pins in DESCRIPTION/lockfile so pak resolves identically), and add a post-setup assertion step that loads stringfish.
  • Regression test: Run the workflow twice (cold and warm cache) and after setup-r-dependencies assert Rscript -e 'stopifnot(requireNamespace("stringfish"))' and compare packageVersion('stringfish') across runs.
  • Suggested diff: posted in this finding's inline review thread.

Summary

REQUEST_CHANGES: PR #208 is titled docs(agents) and its body describes only the AGENTS.md code-owner policy note, yet the diff also adds a 'Prebuild stringfish from source' step at .github/workflows/r.yml:38 that changes every R CMD check run; the RcppParallel 6.2.0/stringfish TBB-mismatch premise and the workaround's effectiveness are unverified (Failed GitHub Check evidence: 'No completed failed GitHub Checks were present when evidence was collected', and no OPENCODE_EXECUTION_RECEIPT exists for the added step at head SHA 38a53ac). Approval sufficiency: insufficient — two source-confirmed P2 blockers at .github/workflows/r.yml:38 plus two unresolved @opencode-agent threads at the same line. Verification posture: no completed check, failed-check log, or execution receipt for the workflow step; verification gap treated as a blocker. Linter/static: not applicable — YAML workflow + Markdown docs only, no lint contract executed. TDD/regression: not applicable to AGENTS.md; the workflow step has no version pins and no post-setup verification assertion. Coverage: Coverage execution evidence reports Result: PASS — test coverage not applicable because no supported changed source files or package manifests were found. Docstring coverage: not applicable — no supported changed source files or package manifests. DAG: base-to-head flowchart of .github/workflows/r.yml placing the unversioned prebuild before r-lib/actions/setup-r-dependencies and R CMD check (see review body). PoC/execution: none — no execution receipt for the prebuild step; recorded as an evidence gap, not approval. DDD/domain: not applicable — no domain code changed. CDD/context: not applicable — no bounded-context code changed. Similar issues: the two unresolved opencode-agent threads at r.yml:38 (unverified correctness premise; floating versions vs pak-managed cache) are independently confirmed from the focused hunks. Claim/concept check: the step comment's claim that 'pak then keeps the installed copy' is unsupported by the workflow's own logic (no pins, no post-setup verification); AGENTS.md:119's code-owner-gate claim is self-consistent and not contradicted by mergeability evidence (mergeStateStatus BLOCKED is a check/policy state, not a merge conflict). Standards search: r-lib/actions/setup-r-dependencies pak-managed library/cache semantics considered; external docs not consulted — stated as a source limitation. Compatibility/convention: no schema/API/db identifiers introduced; new YAML step names are multi-word and reserved-word-safe. Breaking-change/backcompat: every R check run now installs packages before dependency setup; no compatibility regression for the AGENTS.md docs. Implementation completeness: the script body is complete but unverified and unversioned. Performance: two extra package installs per R check run with no caching integration. Developer experience: every R-check PR pays the prebuild cost, and the docs-only PR did not announce the CI change. User experience: non-web interaction surface reviewed = CI workflow behavior and agent-guidance docs. Visual/DOM: not applicable (non-web); reviewed the workflow YAML and Markdown contract instead. Accessibility/i18n: not applicable (no UI); docs are English per Review language evidence. Supply-chain/license: floating CRAN versions of Rcpp/RcppParallel/stringfish without pins create reproducibility/supply-chain risk on every run. Packaging: R package check path is affected by the workflow change. Security/privacy: no secrets, auth, or tenant surfaces introduced; step-security/harden-runner egress audit unchanged.

Adversarial validation

{"status":"failed","probes":[{"path":".github/workflows/r.yml","line":38,"hypothesis":"The unannounced prebuild step mutates every R CMD check run with a TBB-mismatch premise that no trusted check, log, or execution receipt corroborates for head SHA 38a53acec6e527117ab78eb2c1871b15d67b9a28, and the step uses floating package versions that setup-r-dependencies can silently override.","attack_or_counterexample":"Run the repository R check workflow for this head and inspect the step ordering and logs; attempt a cache-hit re-run and check whether the prebuilt source stringfish survives setup-r-dependencies, treating the comment's claim ('stringfish binaries still link the legacy TBB ... fail to load against RcppParallel 6.2.0's oneTBB ... pak then keeps the installed copy') as unverified until a completed check or execution receipt exists.","evidence":"Focused changed hunks show the prebuild step inserted at .github/workflows/r.yml:38 before 'Set up R package dependencies' with unversioned install.packages calls; Failed GitHub Check evidence reports 'No completed failed GitHub Checks were present when evidence was collected' and no OPENCODE_EXECUTION_RECEIPT exists for this step at the head SHA, so neither the failure premise nor the fix effectiveness is corroborated and the workflow mutation is confirmed; source-line-sha256=d2d5fd5e1b629b6e4fcf1e3c187df24a5b982ba7de5fa2debf04d9428931179d","outcome":"confirmed"},{"path":"AGENTS.md","line":119,"hypothesis":"AGENTS.md:119's claim that code-owner review requirements are disabled org-wide is contradicted by the PR mergeability state reporting 'blocked'.","attack_or_counterexample":"Compare the documentation claim against the PR mergeability evidence for head SHA 38a53acec6e527117ab78eb2c1871b15d67b9a28 and check whether 'blocked' implies an active code-owner review gate.","evidence":"PR mergeability evidence reports mergeStateStatus 'blocked' with mergeable 'true', and the review contract states BLOCKED is a branch policy, review, or check state that must not be treated as a merge conflict (only DIRTY or CONFLICTING is a conflict); the blocked state is consistent with required checks such as the pending review gate rather than code-owner reviews, so the docs claim at AGENTS.md:119 is not contradicted and the hypothesis is falsified; source-line-sha256=857323dc769c47e6c1ad28f22493244254b0e326008e07fdfd1129531859b5b9","outcome":"falsified"}],"residual_risk":"If merged as-is, the unversioned prebuild runs before setup-r-dependencies on every R check run; on cache-hit or re-resolution runs the pak-managed library can still restore/reinstall the binary stringfish, so the TBB-mismatch fix may be silently defeated, and no execution receipt verifies the fix for this head SHA. The AGENTS.md policy note is documentation-only and carries no execution risk."}
  • Result: REQUEST_CHANGES

  • Reason: Unresolved reviewer threads and source-confirmed blockers at .github/workflows/r.yml:38: a docs-only PR bundles an unannounced, unverified, unversioned CI workaround that mutates every R check run, with no completed check or execution receipt for this head SHA.

  • Head SHA: 38a53acec6e527117ab78eb2c1871b15d67b9a28

  • Workflow run: 31222594861

  • Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: r.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: r.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file: AGENTS.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: AGENTS.md"]
  R2 --> V2["required checks"]
Loading

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode reviewed the current-head bounded evidence and requested changes before merge.

Findings

1. P2 .github/workflows/r.yml:38 - Unannounced CI workaround bundled into a docs-only PR with an unverified correctness premise

  • Problem: PR title/body describe only the AGENTS.md code-owner gate policy note, yet the diff also adds a 'Prebuild stringfish from source' step to .github/workflows/r.yml that changes every R check run. The packet contains no completed check, failed-check log, or execution receipt for head SHA 38a53ac, so neither the claimed RcppParallel 6.2.0/stringfish TBB load failure nor the fix's effectiveness is corroborated.
  • Root cause: A CI reliability workaround was committed inside a docs PR without its own description or evidence, and without version pins or a tracked follow-up for removal.
  • Fix: Move the workaround into a separate PR (or document it in this PR body with the failing and passing run URLs), pin exact versions (RcppParallel 6.2.0, a fixed stringfish version and an RSPM snapshot) instead of installing latest at run time, and file an issue tracking 'Drop once upstream binaries are rebuilt'.
  • Regression test: Re-run the .github/workflows/r.yml 'Run R CMD check' job on this head and attach the run log showing the SimDesign source build and stringfish load succeeding without TBB symbol errors.
  • Suggested diff: posted in this finding's inline review thread.

2. P2 .github/workflows/r.yml:38 - Prebuild runs before setup-r-dependencies with floating latest versions, so the workaround can be silently defeated

  • Problem: The step installs Rcpp/RcppParallel/stringfish with install.packages before r-lib/actions/setup-r-dependencies runs, and uses no version constraints. setup-r-dependencies manages its own package library/cache and pak-based resolution, so on cache-hit or re-resolution runs the binary stringfish can be restored or reinstalled, and the step's green result does not prove the load-time TBB mismatch is gone; upstream CRAN/P3M releases can also change behavior run to run.
  • Root cause: Dependency resolution is split between install.packages (floating latest) and the subsequent pak step, with the workaround placed at the earliest point in the job where a later cache restore can overwrite it.
  • Fix: Place the source build after setup-r-dependencies (so the source-built stringfish is what R CMD check loads) or disable the action cache for this job, pin versions to an explicit RSPM snapshot, and make the step assert the installed stringfish is a source build (e.g., check packageBuilt metadata).
  • Regression test: Run the R workflow twice (cold cache and warm cache) on this head and confirm both runs load stringfish without the TBB symbol error; verification command when available: re-run .github/workflows/r.yml and inspect the 'Run R CMD check' log for the load/source-build messages.
  • Suggested diff: posted in this finding's inline review thread.

Summary

Inspected changed files .github/workflows/r.yml and AGENTS.md via the inlined current-head packet (head SHA 38a53ac). Approval sufficiency: sufficient for the AGENTS.md policy note (self-consistent with PR title/body), insufficient for the workflow change — no check, receipt, or execution evidence at head supports the RcppParallel 6.2.0 oneTBB / legacy-TBB stringfish premise or the fix. Verification posture: source-trace and focused-hunk inspection only (isolated model; no OPENCODE_EXECUTION_RECEIPT); Failed GitHub Check evidence reports no completed checks for this head, so the CI change is unverified. Linter/static: no lint receipt for workflow YAML; structure inspected (block scalar, balanced quotes, shell continuation). TDD/regression: no tests added; workaround regression risk unmitigated. Coverage: Coverage execution evidence PASS — not applicable (no supported changed source files or package manifests; changed files are workflow YAML + Markdown). Docstring coverage: not applicable per Coverage execution evidence. DAG: source-backed Mermaid flowchart of the r.yml check-job flow at head (setup-r -> Prebuild stringfish step -> setup-r-dependencies -> R CMD check) plus the AGENTS.md doc surface; reflects head from the focused hunk. PoC/execution: none; TBB/oneTBB, P3M source-API-binary, and 'pak keeps the installed copy' claims are unverified assertions in the step comment. DDD/domain: CI/packaging-only surface; no domain-model impact. CDD/context: dependency-resolution context of every r.yml run changes. Similar issues: no similar-issue evidence available offline. Claim/concept check: upstream TBB-mismatch claims lack trusted reference in the packet; treated as unverified. Standards search: offline; none fetched. Compatibility/convention: no new schema/API identifiers; no naming or reserved-word concerns; step inserts an install.packages phase before the repository's setup-r -> setup-r-dependencies convention. Breaking-change/backcompat: all r.yml runs now compile stringfish from source and float latest Rcpp/RcppParallel/stringfish versions without PR-body documentation. Performance: minutes of source compilation added per run; no artifact caching; 'Drop once upstream binaries are rebuilt' is an untracked manual TODO. Developer experience: DX surface = CI contributors — unannounced CI-time increase and new failure mode require manual log reading to diagnose. User experience: UX surface = maintainer/CI consumers — docs note is clear; workflow effect invisible except run logs. Visual/DOM: non-web PR; reviewed the workflow run surface and docs output. Accessibility/i18n: not applicable to workflow+docs change; English consistent. Supply-chain/license: install.packages resolves latest unversioned packages from RSPM/CRAN at run time (reproducibility risk); no license/audit receipts; standard CRAN packages. Packaging: r.yml is the CI packaging surface; prebuild splits dependency resolution between install.packages and pak. Security/privacy: no credentials, secrets, auth, or tenant surfaces touched; HTTPS-only fetches; no new exposure beyond floating-version supply-chain note. Prior bot comments (CodeRabbit 'Review limit reached' and command replies) contain no substantive claims; none need refutation.

Adversarial validation

{"status":"failed","probes":[{"path":".github/workflows/r.yml","line":38,"hypothesis":"The PR is docs-only per its title/body; the added workflow step is unannounced scope drift and no trusted evidence shows the claimed TBB failure or that the step fixes it.","attack_or_counterexample":"Compared the Review language evidence PR title/body (code-owner gate policy only) against the current-head changed-file list (.github/workflows/r.yml + AGENTS.md) and searched the packet for any check log, execution receipt, or reference corroborating the RcppParallel 6.2.0 oneTBB / legacy-TBB stringfish premise.","evidence":"Trusted source trace at .github/workflows/r.yml:38 shows the added 'Prebuild stringfish from source (TBB mismatch with RcppParallel 6.2.0)' step, while Failed GitHub Check evidence reports 'No completed failed GitHub Checks were present' and Coverage execution evidence is 'not applicable', i.e., no trusted outcome corroborates the claimed failure or the fix; source-line-sha256=d2d5fd5e1b629b6e4fcf1e3c187df24a5b982ba7de5fa2debf04d9428931179d","outcome":"confirmed"},{"path":".github/workflows/r.yml","line":48,"hypothesis":"The added run: block is malformed (unbalanced quoting or broken shell continuation), so the workflow YAML or shell command would fail.","attack_or_counterexample":"Parsed the added block from the focused hunk: YAML block scalar preserves the trailing backslash; the two single-quoted -e Rscript expressions are balanced and joined by shell line continuation; the block must terminate before the next step key.","evidence":"Trusted source trace at .github/workflows/r.yml:48 observed the run block terminate cleanly on a blank line immediately followed by the correctly indented '- name: Set up R package dependencies' key, with no quote or continuation defect in the trace; source-line-sha256=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","outcome":"falsified"},{"path":"AGENTS.md","line":119,"hypothesis":"The added AGENTS.md section contradicts the PR intent or existing repository documentation (e.g., states a policy the tree or surrounding docs contradict).","attack_or_counterexample":"Read the added section in the focused hunk and compared it with the PR title/body and the surrounding AGENTS.md structure (Research grounding block and END cwl-agent-guidance marker).","evidence":"Trusted source trace at AGENTS.md:119 shows the section closing with 'CODEOWNERS-based merge gates before then', matching the PR body's 'ON HOLD until the org has multiple maintainers — do not re-enable' and placed cleanly after the existing guidance block, so no doc-to-code or doc-to-intent contradiction was observed in the trace; source-line-sha256=857323dc769c47e6c1ad28f22493244254b0e326008e07fdfd1129531859b5b9","outcome":"falsified"}],"residual_risk":"TBB/oneTBB premise and pak/P3M behavior are unverified offline; setup-r-dependencies cache state may still restore the binary stringfish and defeat the workaround on warm-cache runs; mergeStateStatus 'blocked' cause is not attributable from the evidence (policy/check state, not DIRTY/CONFLICTING); the AGENTS.md 'single maintainer (solo developer)' org claim is attestation that is not independently verifiable from the repository tree."}
  • Result: REQUEST_CHANGES

  • Reason: Unannounced CI workaround in a docs-only PR: the stringfish/TBB prebuild step in .github/workflows/r.yml:38 changes every R check run with no trusted check log, receipt, or reference in the evidence corroborating the claimed failure or the fix, and its placement before setup-r-dependencies plus floating unversioned installs can silently defeat the workaround.

  • Head SHA: 38a53acec6e527117ab78eb2c1871b15d67b9a28

  • Workflow run: 31220201652

  • Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: r.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: r.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file: AGENTS.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: AGENTS.md"]
  R2 --> V2["required checks"]
Loading

Comment thread .github/workflows/r.yml
r-version: release
use-public-rspm: true

# stringfish binaries (CRAN/P3M) still link the legacy TBB from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unannounced CI workaround bundled into a docs-only PR with an unverified correctness premise

  • Location: .github/workflows/r.yml:38
  • Problem: PR title/body describe only the AGENTS.md code-owner gate policy note, yet the diff also adds a 'Prebuild stringfish from source' step to .github/workflows/r.yml that changes every R check run. The packet contains no completed check, failed-check log, or execution receipt for head SHA 38a53ac, so neither the claimed RcppParallel 6.2.0/stringfish TBB load failure nor the fix's effectiveness is corroborated.
  • Root cause: A CI reliability workaround was committed inside a docs PR without its own description or evidence, and without version pins or a tracked follow-up for removal.
  • Fix: Move the workaround into a separate PR (or document it in this PR body with the failing and passing run URLs), pin exact versions (RcppParallel 6.2.0, a fixed stringfish version and an RSPM snapshot) instead of installing latest at run time, and file an issue tracking 'Drop once upstream binaries are rebuilt'.
  • Regression test: Re-run the .github/workflows/r.yml 'Run R CMD check' job on this head and attach the run log showing the SimDesign source build and stringfish load succeeding without TBB symbol errors.

Suggested diff

```diff
-      # stringfish binaries (CRAN/P3M) still link the legacy TBB from
-      # RcppParallel <6.2 and fail to load against RcppParallel 6.2.0's oneTBB,
-      # which breaks the SimDesign source build inside setup-r-dependencies.
-      # Pre-build stringfish from real CRAN source (P3M serves binaries even
-      # via its source API); pak then keeps the installed copy. Drop once
-      # upstream binaries are rebuilt.
-      - name: Prebuild stringfish from source (TBB mismatch with RcppParallel 6.2.0)
-        run: |
-          Rscript -e 'install.packages(c("Rcpp", "RcppParallel"), repos = Sys.getenv("RSPM", unset = "https://cloud.r-project.org"))' \
-                  -e 'install.packages("stringfish", type = "source", repos = "https://cloud.r-project.org")'
-

Comment thread .github/workflows/r.yml
r-version: release
use-public-rspm: true

# stringfish binaries (CRAN/P3M) still link the legacy TBB from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Prebuild runs before setup-r-dependencies with floating latest versions, so the workaround can be silently defeated

  • Location: .github/workflows/r.yml:38
  • Problem: The step installs Rcpp/RcppParallel/stringfish with install.packages before r-lib/actions/setup-r-dependencies runs, and uses no version constraints. setup-r-dependencies manages its own package library/cache and pak-based resolution, so on cache-hit or re-resolution runs the binary stringfish can be restored or reinstalled, and the step's green result does not prove the load-time TBB mismatch is gone; upstream CRAN/P3M releases can also change behavior run to run.
  • Root cause: Dependency resolution is split between install.packages (floating latest) and the subsequent pak step, with the workaround placed at the earliest point in the job where a later cache restore can overwrite it.
  • Fix: Place the source build after setup-r-dependencies (so the source-built stringfish is what R CMD check loads) or disable the action cache for this job, pin versions to an explicit RSPM snapshot, and make the step assert the installed stringfish is a source build (e.g., check packageBuilt metadata).
  • Regression test: Run the R workflow twice (cold cache and warm cache) on this head and confirm both runs load stringfish without the TBB symbol error; verification command when available: re-run .github/workflows/r.yml and inspect the 'Run R CMD check' log for the load/source-build messages.

Suggested diff

```diff
       - name: Set up R package dependencies
         uses: r-lib/actions/setup-r-dependencies@d3c5be51b12e724e68f33216ca3c148b66d5f0b6
         with:
           extra-packages: any::rcmdcheck
           needs: check
 
+      # Move the stringfish source build AFTER dependency setup so the
+      # library cache/pak cannot restore the legacy-TBB binary over it.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode reviewed the current-head bounded evidence and requested changes before merge.

Findings

1. P2 .github/workflows/r.yml:38 - Unannounced CI workaround bundled into a docs-only PR with an unverified correctness premise

  • Problem: PR title 'docs(agents): code-owner gates disabled org-wide until multiple maintainers exist' and body describe only the AGENTS.md policy note, yet the diff also adds a 'Prebuild stringfish from source (TBB mismatch with RcppParallel 6.2.0)' step that changes every R check run. Failed GitHub Check evidence reports 'No completed failed GitHub Checks were present when evidence was collected' and no OPENCODE_EXECUTION_RECEIPT exists for the step at head SHA 38a53ac, so neither the claimed RcppParallel 6.2.0/stringfish TBB load failure nor the workaround's effectiveness is corroborated.
  • Root cause: Docs-only PR scope bundled an unverified CI workaround without announcing it in the title/body and without attaching completed check evidence for the head SHA.
  • Fix: Split the workflow change into a separate CI-focused PR, or update this PR's title/body to announce the workflow change and attach the completed successful R check run for this head SHA; keep AGENTS.md documentation change separate if preferred.
  • Regression test: Run the repository R check workflow (r-lib/actions/setup-r-dependencies then r-lib/actions/check-r-package) for head SHA 38a53ac and attach the completed check log proving R CMD check passes with the prebuilt stringfish.
  • Suggested diff: posted in this finding's inline review thread.

2. P2 .github/workflows/r.yml:38 - Floating-version prebuild runs before setup-r-dependencies and can be silently defeated on cache-hit runs

  • Problem: The step installs Rcpp, RcppParallel, and stringfish via install.packages with no version constraints and runs before r-lib/actions/setup-r-dependencies, which manages its own pak-based library and cache. The step comment asserts 'pak then keeps the installed copy', but the workflow pins nothing and verifies nothing after dependency setup; on a cache-hit or re-resolution run setup-r-dependencies can restore or reinstall the P3M binary stringfish, so the step's green result does not prove the load-time TBB mismatch is gone.
  • Root cause: No version pins, no guarantee the prebuilt library is the library setup-r-dependencies resolves, and no post-setup verification step that asserts stringfish loads from the source build.
  • Fix: Pin exact versions (RcppParallel 6.2.0 and the matching stringfish release), install into the same library target that setup-r-dependencies uses (or declare the pins in DESCRIPTION/lockfile so pak resolves identically), and add a post-setup assertion step that loads stringfish.
  • Regression test: Run the workflow twice (cold and warm cache) and after setup-r-dependencies assert Rscript -e 'stopifnot(requireNamespace("stringfish"))' and compare packageVersion('stringfish') across runs.
  • Suggested diff: posted in this finding's inline review thread.

Summary

REQUEST_CHANGES: PR #208 is titled docs(agents) and its body describes only the AGENTS.md code-owner policy note, yet the diff also adds a 'Prebuild stringfish from source' step at .github/workflows/r.yml:38 that changes every R CMD check run; the RcppParallel 6.2.0/stringfish TBB-mismatch premise and the workaround's effectiveness are unverified (Failed GitHub Check evidence: 'No completed failed GitHub Checks were present when evidence was collected', and no OPENCODE_EXECUTION_RECEIPT exists for the added step at head SHA 38a53ac). Approval sufficiency: insufficient — two source-confirmed P2 blockers at .github/workflows/r.yml:38 plus two unresolved @opencode-agent threads at the same line. Verification posture: no completed check, failed-check log, or execution receipt for the workflow step; verification gap treated as a blocker. Linter/static: not applicable — YAML workflow + Markdown docs only, no lint contract executed. TDD/regression: not applicable to AGENTS.md; the workflow step has no version pins and no post-setup verification assertion. Coverage: Coverage execution evidence reports Result: PASS — test coverage not applicable because no supported changed source files or package manifests were found. Docstring coverage: not applicable — no supported changed source files or package manifests. DAG: base-to-head flowchart of .github/workflows/r.yml placing the unversioned prebuild before r-lib/actions/setup-r-dependencies and R CMD check (see review body). PoC/execution: none — no execution receipt for the prebuild step; recorded as an evidence gap, not approval. DDD/domain: not applicable — no domain code changed. CDD/context: not applicable — no bounded-context code changed. Similar issues: the two unresolved opencode-agent threads at r.yml:38 (unverified correctness premise; floating versions vs pak-managed cache) are independently confirmed from the focused hunks. Claim/concept check: the step comment's claim that 'pak then keeps the installed copy' is unsupported by the workflow's own logic (no pins, no post-setup verification); AGENTS.md:119's code-owner-gate claim is self-consistent and not contradicted by mergeability evidence (mergeStateStatus BLOCKED is a check/policy state, not a merge conflict). Standards search: r-lib/actions/setup-r-dependencies pak-managed library/cache semantics considered; external docs not consulted — stated as a source limitation. Compatibility/convention: no schema/API/db identifiers introduced; new YAML step names are multi-word and reserved-word-safe. Breaking-change/backcompat: every R check run now installs packages before dependency setup; no compatibility regression for the AGENTS.md docs. Implementation completeness: the script body is complete but unverified and unversioned. Performance: two extra package installs per R check run with no caching integration. Developer experience: every R-check PR pays the prebuild cost, and the docs-only PR did not announce the CI change. User experience: non-web interaction surface reviewed = CI workflow behavior and agent-guidance docs. Visual/DOM: not applicable (non-web); reviewed the workflow YAML and Markdown contract instead. Accessibility/i18n: not applicable (no UI); docs are English per Review language evidence. Supply-chain/license: floating CRAN versions of Rcpp/RcppParallel/stringfish without pins create reproducibility/supply-chain risk on every run. Packaging: R package check path is affected by the workflow change. Security/privacy: no secrets, auth, or tenant surfaces introduced; step-security/harden-runner egress audit unchanged.

Adversarial validation

{"status":"failed","probes":[{"path":".github/workflows/r.yml","line":38,"hypothesis":"The unannounced prebuild step mutates every R CMD check run with a TBB-mismatch premise that no trusted check, log, or execution receipt corroborates for head SHA 38a53acec6e527117ab78eb2c1871b15d67b9a28, and the step uses floating package versions that setup-r-dependencies can silently override.","attack_or_counterexample":"Run the repository R check workflow for this head and inspect the step ordering and logs; attempt a cache-hit re-run and check whether the prebuilt source stringfish survives setup-r-dependencies, treating the comment's claim ('stringfish binaries still link the legacy TBB ... fail to load against RcppParallel 6.2.0's oneTBB ... pak then keeps the installed copy') as unverified until a completed check or execution receipt exists.","evidence":"Focused changed hunks show the prebuild step inserted at .github/workflows/r.yml:38 before 'Set up R package dependencies' with unversioned install.packages calls; Failed GitHub Check evidence reports 'No completed failed GitHub Checks were present when evidence was collected' and no OPENCODE_EXECUTION_RECEIPT exists for this step at the head SHA, so neither the failure premise nor the fix effectiveness is corroborated and the workflow mutation is confirmed; source-line-sha256=d2d5fd5e1b629b6e4fcf1e3c187df24a5b982ba7de5fa2debf04d9428931179d","outcome":"confirmed"},{"path":"AGENTS.md","line":119,"hypothesis":"AGENTS.md:119's claim that code-owner review requirements are disabled org-wide is contradicted by the PR mergeability state reporting 'blocked'.","attack_or_counterexample":"Compare the documentation claim against the PR mergeability evidence for head SHA 38a53acec6e527117ab78eb2c1871b15d67b9a28 and check whether 'blocked' implies an active code-owner review gate.","evidence":"PR mergeability evidence reports mergeStateStatus 'blocked' with mergeable 'true', and the review contract states BLOCKED is a branch policy, review, or check state that must not be treated as a merge conflict (only DIRTY or CONFLICTING is a conflict); the blocked state is consistent with required checks such as the pending review gate rather than code-owner reviews, so the docs claim at AGENTS.md:119 is not contradicted and the hypothesis is falsified; source-line-sha256=857323dc769c47e6c1ad28f22493244254b0e326008e07fdfd1129531859b5b9","outcome":"falsified"}],"residual_risk":"If merged as-is, the unversioned prebuild runs before setup-r-dependencies on every R check run; on cache-hit or re-resolution runs the pak-managed library can still restore/reinstall the binary stringfish, so the TBB-mismatch fix may be silently defeated, and no execution receipt verifies the fix for this head SHA. The AGENTS.md policy note is documentation-only and carries no execution risk."}
  • Result: REQUEST_CHANGES

  • Reason: Unresolved reviewer threads and source-confirmed blockers at .github/workflows/r.yml:38: a docs-only PR bundles an unannounced, unverified, unversioned CI workaround that mutates every R check run, with no completed check or execution receipt for this head SHA.

  • Head SHA: 38a53acec6e527117ab78eb2c1871b15d67b9a28

  • Workflow run: 31222594861

  • Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: r.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: r.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file: AGENTS.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: AGENTS.md"]
  R2 --> V2["required checks"]
Loading

Comment thread .github/workflows/r.yml
r-version: release
use-public-rspm: true

# stringfish binaries (CRAN/P3M) still link the legacy TBB from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unannounced CI workaround bundled into a docs-only PR with an unverified correctness premise

  • Location: .github/workflows/r.yml:38
  • Problem: PR title 'docs(agents): code-owner gates disabled org-wide until multiple maintainers exist' and body describe only the AGENTS.md policy note, yet the diff also adds a 'Prebuild stringfish from source (TBB mismatch with RcppParallel 6.2.0)' step that changes every R check run. Failed GitHub Check evidence reports 'No completed failed GitHub Checks were present when evidence was collected' and no OPENCODE_EXECUTION_RECEIPT exists for the step at head SHA 38a53ac, so neither the claimed RcppParallel 6.2.0/stringfish TBB load failure nor the workaround's effectiveness is corroborated.
  • Root cause: Docs-only PR scope bundled an unverified CI workaround without announcing it in the title/body and without attaching completed check evidence for the head SHA.
  • Fix: Split the workflow change into a separate CI-focused PR, or update this PR's title/body to announce the workflow change and attach the completed successful R check run for this head SHA; keep AGENTS.md documentation change separate if preferred.
  • Regression test: Run the repository R check workflow (r-lib/actions/setup-r-dependencies then r-lib/actions/check-r-package) for head SHA 38a53ac and attach the completed check log proving R CMD check passes with the prebuilt stringfish.

Suggested diff

-      # stringfish binaries (CRAN/P3M) still link the legacy TBB from
-      # RcppParallel <6.2 and fail to load against RcppParallel 6.2.0's oneTBB,
-      # which breaks the SimDesign source build inside setup-r-dependencies.
-      # Pre-build stringfish from real CRAN source (P3M serves binaries even
-      # via its source API); pak then keeps the installed copy. Drop once
-      # upstream binaries are rebuilt.
-      - name: Prebuild stringfish from source (TBB mismatch with RcppParallel 6.2.0)
-        run: |
-          Rscript -e 'install.packages(c("Rcpp", "RcppParallel"), repos = Sys.getenv("RSPM", unset = "https://cloud.r-project.org"))' \
-                  -e 'install.packages("stringfish", type = "source", repos = "https://cloud.r-project.org")'
-

Comment thread .github/workflows/r.yml
r-version: release
use-public-rspm: true

# stringfish binaries (CRAN/P3M) still link the legacy TBB from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Floating-version prebuild runs before setup-r-dependencies and can be silently defeated on cache-hit runs

  • Location: .github/workflows/r.yml:38
  • Problem: The step installs Rcpp, RcppParallel, and stringfish via install.packages with no version constraints and runs before r-lib/actions/setup-r-dependencies, which manages its own pak-based library and cache. The step comment asserts 'pak then keeps the installed copy', but the workflow pins nothing and verifies nothing after dependency setup; on a cache-hit or re-resolution run setup-r-dependencies can restore or reinstall the P3M binary stringfish, so the step's green result does not prove the load-time TBB mismatch is gone.
  • Root cause: No version pins, no guarantee the prebuilt library is the library setup-r-dependencies resolves, and no post-setup verification step that asserts stringfish loads from the source build.
  • Fix: Pin exact versions (RcppParallel 6.2.0 and the matching stringfish release), install into the same library target that setup-r-dependencies uses (or declare the pins in DESCRIPTION/lockfile so pak resolves identically), and add a post-setup assertion step that loads stringfish.
  • Regression test: Run the workflow twice (cold and warm cache) and after setup-r-dependencies assert Rscript -e 'stopifnot(requireNamespace("stringfish"))' and compare packageVersion('stringfish') across runs.

Suggested diff

+      - name: Verify stringfish load after dependency setup
+        run: Rscript -e 'stopifnot(requireNamespace("stringfish"))'

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