fix: add org-scoped ruleset support

[vish-dawange]

No description provided.

[Copilot]

Pull request overview

Adds organization-scoped ruleset reconciliation for suborganization configurations while preserving repository-scoped behavior.

Changes:

• Aggregates top-level and suborganization rulesets.
• Resolves ruleset scope and applies repository-property targeting.
• Suppresses organization-scoped rulesets from per-repository synchronization.

Show a summary per file

File

Findings

lib/settings.js

Critical: Existing repo-scoped rulesets are not removed when migrating to org scope (line 2439, 4 votes); cleanup is skipped when the last org-scoped configuration is removed (line 2215, 2 votes); targeting semantics can omit repositories (line 2197, 2 votes); conflicting selectors can produce invalid payloads (line 2207, 1 vote). Moderate: Org-scoped NOP results can be filtered incorrectly (line 2221, 3 votes); suborg strips are bypassed (line 2194, 3 votes); reconciliation runs redundantly per suborg (line 2160, 3 votes); unsupported scopes are not validated (line 2150, 3 votes). Nit: Missing focused tests for the new orchestration (line 2178, 2 votes).

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (2)

lib/settings.js:2160

• syncOrgLevelRulesets() is only called from updateAll(), but Settings.sync()—the normal repo webhook path—calls updateRepos() directly. If a repo config change gives a repo a property matching an org-scoped suborg, re-evaluation reaches this suppression and no org-level reconciliation runs, so the rule may not be created until another full or suborg sync. Invoke the org reconciliation from that path as well.

    await this.syncOrgLevelRulesets()

lib/settings.js:2150

• The new user-facing ruleset_scope and the suborg rulesets section it controls are not declared under properties in schema/suborgs.json; ruleset_scope is also absent from schema/settings.json. Schema-backed editor/validation tooling therefore cannot describe this feature, and the generated dereferenced schemas will not expose it. Add the source schema fields and regenerate the dereferenced outputs.

    const requested = (subOrgData && subOrgData.ruleset_scope) || (this.config && this.config.ruleset_scope) || 'repo'

• Files reviewed: 1/1 changed files
• Comments generated: 9
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

lib/settings.js:2248

• In NOP mode, org-level ruleset results are always tagged fromSubOrg = true, which makes the base-config drift filter treat any org-level rulesets output as suborg-derived whenever there are changed repos/suborgs. This can surface unrelated org-level ruleset drift in PR dry-runs even when no suborg configs changed. Tagging should be conditional so repo-only changes don’t keep org-level rulesets results by accident.

      if (this.nop && Array.isArray(res)) {
        res.forEach(r => { 
          if (r) {
            r.repo = `${this.repo.owner} (org)`
            r.fromSubOrg = true
          }
        })

lib/settings.js:2157

• New behavior (org-scoped suborg rulesets via ruleset_scope and syncOrgLevelRulesets) is introduced here, but there are no existing unit/integration tests exercising ruleset_scope, getEffectiveRulesetScope, or syncOrgLevelRulesets (search in test/ shows no references). Adding tests would help prevent regressions in org-vs-repo ruleset behavior and migration cleanup.

  // Resolves whether a suborg's rulesets should be applied as one shared
  // org-level ruleset ('org') or per-repo ('repo', the original behavior).
  // Precedence: the suborg's own `ruleset_scope` wins if set; otherwise falls
  // back to the org-wide default `ruleset_scope` in org-settings.yml (admin-
  // team controlled); otherwise defaults to 'repo' for backward compatibility.
  // 'org' scope requires `suborgproperties` to build the repository_property
  // filter from - without it, falls back to 'repo' so rulesets from suborgs
  // matched via suborgteams/suborgrepos aren't silently dropped.
  getEffectiveRulesetScope (subOrgData) {

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Lite

[decyjphr]

@vish-dawange you can regenerate these by running script/build-schema