v3/implementation/adrs/ADR-065-external-pr-review-v3523.md
In Progress — Review started 2026-03-17
2026-03-17
Ruflo v3.5.22 achieved 100% capability audit (76/76 checks, 259 MCP tools). With the project gaining traction (5,900+ commits, growing community), external contributors are submitting PRs that fix real bugs and add valuable features. This ADR documents the review process, verdicts, and merge plan for the first batch of community PRs.
Each PR is evaluated on 4 axes:
| Axis | Weight | What We Check |
|---|---|---|
| Capability | 30% | Does it fix a real bug or add real value? |
| Security | 30% | Path traversal, injection, poisoned data, module hijacking |
| Regression | 25% | Breaks existing behavior? Conflicts with v3.5.22? |
| Code Quality | 15% | Tests, idiomatic patterns, file size, consistency |
Math.max(cpuCount * 0.8, 2.0) + cgroup v1/v2 container detectionNUMERIC_RE. sanitize() strips control chars. Upper bound of 1000 prevents Infinity bypass.initializeWorkerStates() silently overrides config.json values from stale daemon-state.json. Add guard: skip restoring resourceThresholds when constructor received explicit values. (R2) Add upper bound check to readDaemonConfigFromFile for consistency. (R3) .unref() the 30s backoff timer to prevent blocking shutdown..claude-flow/routing-outcomes.json, 500-entry FIFO capagent/task before JSON persistence. Add length + character whitelist: agent.length > 100 || !/^[a-zA-Z0-9_-]+$/.test(agent).getMergedTaskPatterns() out of .map() callback — currently N file reads per route call. (2) Add agent name validation. (3) Replace regex dir extraction with dirname(). (4) Consider in-memory cache for loadRoutingOutcomes().const string literal, no user input. Existing precedent in ruvllm-bridge.ts:310..default || module pattern for CJS environmentsnode -e with hardcoded string literals only. Complementary to existing files field (runtime verification vs tarball inclusion)..d.js → .d.ts across 8 subpath exports. Old paths pointed to nonexistent files (TypeScript never emits .d.js). Pure metadata fix, no runtime changes.@ruvector/attention using createRequire. Correct approach, no security concerns.MultiHeadAttention and LinearAttention (imported by benchmarks but dropped by PR). (2) Add comment explaining computeRaw alias exists for backward compat.require('os') → import os from 'node:os'. Correct ESM fix with regression test. No env injection risk.getCoreVersion() — marginally improves enforcement consistency. || → ?? for numeric priority is a strict correctness improvement (honors explicit priority: 0).| PR | Reason |
|---|---|
| #1357 | launch.json — dev config, not needed in repo |
| #1350, #1324 | Duplicate MiniMax provider PRs — need dedup |
| #1325 | NextAuth.js example — unrelated to core |
| #1319, #1317, #1305, #1304 | Docs/branding — low priority |
Merge in dependency order, clean PRs first:
Phase 1 — Clean accepts (no changes needed):
#1346 (TS2307 fix, 3 lines)
#1314 (prepublish guard, 3 lines)
#1341 (hooks type exports, 8 lines)
#1337 (benchmark ESM, 31 lines)
#1336 (plugin priority, 13 lines)
Phase 2 — Accept with changes (request fixes, then merge):
#1353 (daemon CPU — request R1/R2/R3 fixes from @luis-b-o)
#1311 (routing loop — request input validation + hoist from @eswann)
#1338 (attention interop — request missing wrappers from @Gujiassh)
Phase 3 — Rejected (request rebase):
#1334 (ESM/CJS ruvector — needs rebase onto post-v3.5.22 main)
After merging accepted PRs:
tsc build — 0 errorsReviews complete. 9 PRs reviewed by parallel agent swarm on 2026-03-17.
| Verdict | Count | PRs |
|---|---|---|
| ACCEPT | 5 | #1346, #1314, #1341, #1337, #1336 |
| ACCEPT WITH CHANGES | 3 | #1353, #1311, #1338 |
| REJECT (rebase) | 1 | #1334 |
Phase 1 merge approved. 5 clean PRs ready for immediate merge. Phase 2 pending. Review comments posted; awaiting contributor fixes. Phase 3 blocked. Contributor must rebase onto post-v3.5.22 main.